[bazel] Add individual test targets, broken up by CPU and GPU

Trimmed down testing output:

$ bazel test //tests/... --config=gl
//tests:GPUDrawPathTest                 PASSED in 0.6s
//tests:MatrixProcsTest                 PASSED in 0.1s

$ bazel test //tests/... --config=cpu
//tests:GPUDrawPathTest                 PASSED in 0.0s
//tests:MatrixProcsTest                 PASSED in 0.1s

# Make small change to tests/MatrixProcsTest.cpp

$ bazel test //tests/... --config=cpu
//tests:GPUDrawPathTest        (cached) PASSED in 0.0s
//tests:MatrixProcsTest                 PASSED in 0.1s

# Compile and run CPU tests remotely on Linux RBE instances;
bazel test //tests/... --config=linux_rbe --config=cpu --remote_download_minimal
INFO: Elapsed time: 54.482s, Critical Path: 45.77s
INFO: 1320 processes: 112 remote cache hit, 1583 remote.
INFO: Build completed, 1 test FAILED, 1320 total actions
//tests:TypefaceMacTest                 SKIPPED
//tests:AAClipTest                      PASSED in 2.2s
...
//tests:YUVTest                         PASSED in 1.7s
//tests:TypefaceTest                    FAILED in 1.7s

(I think the TypefaceTest is failing remotely because the
RBE instances might not have any fonts installed in the
Docker container).

All test files in the tests subfolder (but not //tests/graphite
or //tests/sksl) are categorized as Ganesh GPU tests or CPU tests.
This categorization is based on the question "Do we need a Ganesh
GPU backend to compile the test file or not?".

These are further grouped into tests that need similar features
enabled, for example CODEC_TESTS and PDF_TESTS. Features are
enabled via a new transition rule cc_test_with_flags, which is
very similar to cc_binary_with_flags.

These groups are turned into Bazel targets using macros defined
in //tests/test.bzl (basically just a for loop and some select
statements).

Bazel does not let us define some tests when certain --config
options are made (at least, not with macros), so we always
define every test target. We do use select() statements to
decide if this test should do something (that is, run Skia and
do assertions) or not (just return 0). This is based off one of
the options passed into --config. For example --config=cpu
only *really* runs the tests which do not need a GPU backend
(e.g. CODEC_TESTS, PDF_TESTS, and more). --config=gl compiles
in the Ganesh GL backend and runs the tests that require a
GPU backend.

bazel/buildrc has some configs for testing one GPU backend
at a time, as this is how we currently have our CI jobs
organized. Local developers could define their own settings
with multiple backends as necessary and use that for a
--config instead.

Known Limitations:
 - CPU tests all pass for me locally. One fails remotely.
 - Some (~12) GPU tests fail locally. Have not delved into why yet.
 - We do not (yet) enforce IWYU on all the tests. I would like to,
   but that's for another time due to the large amount of files
   that would need changing.
 - Have only run the tests on a Linux box so far.

Suggested Review Order:
 - //tests/test.bzl, noting the select() statements that
   compile BazelTestRunner.cpp etc or BazelNoopRunner.cpp
 - //tests/BUILD.bazel to see the breakdown and usages of
   the macros.
 - //tests/Bazel*Runner.cpp, noting some similarities to how
   dm/DM.cpp works (it was what I looked at when building this).
 - //bazel/cc_test_with_flags.bzl, noting how it uses the same
   list from cc_binary_with_flags.bzl.
 - .bazelproject, which makes integration with Intellij a little
   bit easier (but this could use some tweaks).
 - All the other BUILD.bazel files, noting how we needed a few
   more settings to make all the tests compile correctly
   (e.g. //src/lazy:enable_discardable_memory and the fontmgrs).
 - All other files.

Change-Id: I39d4dcbf03e45d31c3d9bea3a4eb04cd08cb0b05
Bug: skia:13758
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/583247
Reviewed-by: Ben Wagner <[email protected]>
Reviewed-by: Leandro Lovisolo <[email protected]>

.bazelproject

@@ -0,0 +1,19 @@
+directories:
+  .
+  -bin
+  -out
+  -node_modules
+  -third_party/externals
+
+# Automatically includes all relevant targets under the 'directories' above
+derive_targets_from_directories: false
+
+targets:
+  # If source code isn't resolving, add additional targets that compile it here
+  //example:hello_world_vulkan
+  //example:hello_world_gl
+additional_languages:
+  # Uncomment any additional languages you want supported
+  javascript
+  python
+  typescript

.bazelrc

@@ -43,6 +43,8 @@ build --flag_alias=include_encoder=//src/images:include_encoder
 build --flag_alias=include_fontmgr=//bazel/common_config_settings:include_fontmgr
 build --flag_alias=with_gl_standard=//src/gpu:with_gl_standard
 
+build --flag_alias=disable_discardable_memory=no//src/lazy:enable_discardable_memory
+build --flag_alias=enable_discardable_memory=//src/lazy:enable_discardable_memory
 build --flag_alias=disable_effect_serialization=no//bazel/common_config_settings:enable_effect_serialization
 build --flag_alias=enable_effect_serialization=//bazel/common_config_settings:enable_effect_serialization
 build --flag_alias=disable_gpu_test_utils=no//src/gpu:enable_gpu_test_utils
@@ -65,6 +67,8 @@ build --flag_alias=disable_tracing=no//bazel/common_config_settings:enable_traci
 build --flag_alias=enable_tracing=//bazel/common_config_settings:enable_tracing
 build --flag_alias=disable_vma=no//src/gpu:use_vulkan_memory_allocator
 build --flag_alias=enable_vma=//src/gpu:use_vulkan_memory_allocator
+build --flag_alias=with_default_global_memory_pool=//src/lazy:use_default_global_memory_pool
+build --flag_alias=with_no_global_memory_pool=no//src/lazy:use_default_global_memory_pool
 build --flag_alias=with_harfbuzz=//bazel/common_config_settings:use_harfbuzz
 build --flag_alias=with_no_harfbuzz=no//bazel/common_config_settings:use_harfbuzz
 build --flag_alias=with_icu=//bazel/common_config_settings:use_icu

BUILD.gn

@@ -536,6 +536,8 @@ optional("fontmgr_win") {
 
   public = [ "include/ports/SkTypeface_win.h" ]
   sources = [
+    "include/ports/SkFontMgr_indirect.h",
+    "include/ports/SkRemotableFontMgr.h",
     "src/fonts/SkFontMgr_indirect.cpp",
     "src/ports/SkFontMgr_win_dw.cpp",
     "src/ports/SkScalerContext_win_dw.cpp",

PRESUBMIT.py

@@ -250,7 +250,8 @@ def _CheckBazelBUILDFiles(input_api, output_api):
     is_bazel = affected_file_path.endswith('BUILD.bazel')
     # This list lines up with the one in autoroller_lib.py (see G3).
     excluded_paths = ["infra/", "bazel/rbe/", "bazel/external/", "bazel/common_config_settings/",
-                      "modules/canvaskit/go/", "experimental/", "bazel/platform", "third_party/"]
+                      "modules/canvaskit/go/", "experimental/", "bazel/platform", "third_party/",
+                      "tests/", "resources/"]
     is_excluded = any(affected_file_path.startswith(n) for n in excluded_paths)
     if is_bazel and not is_excluded:
       with open(affected_file_path, 'r') as file:

bazel/BUILD.bazel

@@ -52,6 +52,9 @@ GENERAL_DEFINES = [
 }) + select({
     "//src/shaders:legacy_shader_context_true": [],  # This is the default in SkTypes.h
     "//src/shaders:legacy_shader_context_false": ["SK_DISABLE_LEGACY_SHADERCONTEXT"],
+}) + select({
+    "//src/lazy:enable_discardable_memory_true": ["SK_USE_DISCARDABLE_SCALEDIMAGECACHE"],
+    "//src/lazy:enable_discardable_memory_false": [],
 })
 
 GPU_DEFINES = select_multi({

bazel/Makefile

@@ -71,9 +71,13 @@ iwyu_rbe:
 		--config=enforce_iwyu --remote_download_minimal --nobuild_runfile_manifests
 	bazelisk build //modules/skottie:skottie_tool_gpu --config=for_linux_x64_with_rbe \
 		--config=enforce_iwyu --remote_download_minimal --nobuild_runfile_manifests
+	bazelisk build //tests/... --config=for_linux_x64_with_rbe \
+		--config=enforce_iwyu --remote_download_minimal --nobuild_runfile_manifests
 
 iwyu:
 	bazelisk build //:skia_public --config=enforce_iwyu \
 		--nobuild_runfile_manifests
 	bazelisk build //modules/skottie:skottie_tool_gpu --config=enforce_iwyu \
 		--nobuild_runfile_manifests
+	bazelisk build //tests/... --config=enforce_iwyu \
+		--nobuild_runfile_manifests

bazel/buildrc

@@ -10,6 +10,10 @@
 # For example, a default output directory might look like "k8-fastbuild-ST-6a54c1377847".
 # Inside this output directory is a subfolder for the target (e.g. executable) name.
 #
+# Note that multiple definitions of a config are allowed - in this event, they are added together.
+# This is handy if we need to comment on why certain settings are necessary. Otherwise, escaping
+# the trailing newline (and thus having a multi-line definition) is preferred for brevity.
+#
 # Notably, the flags that we turn on and off via //bazel/common_config_settings do not affect
 # the output directory. The output directory is used to store compiled object files (.o files)
 # and generated dependency files (e.g. the output of clang --write-dependencies [5]), so having
@@ -86,6 +90,7 @@ build:enforce_iwyu --features=skia_enforce_iwyu --cc_output_directory_tag=iwyu \
                    --compilation_mode=dbg --keep_going \
                    --with_gl_standard=gl_standard --gpu_backend=gl_backend \
                    --gpu_backend=vulkan_backend --gpu_backend=dawn_backend \
+                   --enable_gpu_test_utils \
                    --include_fontmgr=custom_directory_fontmgr --include_fontmgr=custom_embedded_fontmgr \
                    --include_fontmgr=custom_empty_fontmgr --fontmgr_factory=custom_directory_fontmgr_factory \
                    --include_decoder=avif_decode_codec --include_decoder=gif_decode_codec \
@@ -97,3 +102,24 @@ build:enforce_iwyu --features=skia_enforce_iwyu --cc_output_directory_tag=iwyu \
                    --with_harfbuzz --with_icu \
                    --enable_sksl_tracing \
                    --enable_svg_canvas --enable_pdf_backend
+
+
+build:cpu_only --cc_output_directory_tag=cpu_tests
+
+build:gl_ganesh --enable_gpu_test_utils --gpu_backend=gl_backend \
+                 --cc_output_directory_tag=gl_ganesh
+# We need to have this environment variable set when testing our Ganesh GL backend on Unix,
+# otherwise, we get "Failed to open X display." and connect make a GL context for testing.
+build:gl_ganesh --action_env=DISPLAY=:1
+
+build:vulkan_ganesh --enable_gpu_test_utils --gpu_backend=vulkan_backend \
+                    --cc_output_directory_tag=vulkan_ganesh
+
+build:dawn_ganesh --enable_gpu_test_utils --gpu_backend=dawn_backend \
+                    --cc_output_directory_tag=dawn_ganesh
+
+# Short-hand aliases
+build:cpu --config=cpu_only
+build:gl --config=gl_ganesh
+build:vk --config=vulkan_ganesh
+build:dawn --config=dawn_ganesh

bazel/cc_binary_with_flags.bzl

@@ -13,6 +13,8 @@ load("//bazel:copts.bzl", "DEFAULT_COPTS")
 _bool_flags = [
     "//bazel/common_config_settings:use_harfbuzz",
     "//bazel/common_config_settings:use_icu",
+    "//src/lazy:enable_discardable_memory",
+    "//src/lazy:use_default_global_memory_pool",
     "//src/gpu:enable_gpu_test_utils",
     "//src/pdf:enable_pdf_backend",
     "//src/sksl:enable_sksl",
@@ -35,9 +37,14 @@ _string_list_flags = [
 
 # These are the flags that we support setting via set_flags
 _flags = _bool_flags + _string_flags + _string_list_flags
+_short_flags = [long_flag.split(":")[1] for long_flag in _flags]
 
 def _flag_transition_impl(settings, attr):
     rv = {}
+    for flag in attr.set_flags:
+        if flag not in _short_flags:
+            fail("unknown flag " + flag)
+
     for key in settings:
         # Get the short form of the name. This the short form used as the keys in the
         # set_flags dictionary.
@@ -58,7 +65,7 @@ def _flag_transition_impl(settings, attr):
                 rv[key] = flag_setting  # we know flag_setting is a string (e.g. the default).
         elif key in _bool_flags:
             if type(flag_setting) == "list":
-                rv[key] = flag_setting[0] == "True"
+                rv[key] = flag_setting[0].lower() == "true"
             else:
                 rv[key] = flag_setting  # flag_setting will be a boolean, the default
     return rv

bazel/cc_test_with_flags.bzl

@@ -0,0 +1,86 @@
+"""
+THIS IS THE EXTERNAL-ONLY VERSION OF THIS FILE. G3 HAS ITS OWN.
+
+This file contains a way to set flags from BUILD.bazel instead of requiring users to set them from
+the CLI. This allows us to have tests that build with a specific set of features (e.g. the pdf
+backend or the codecs) without having a large number of flags to set (easier to forget).
+
+It uses the same core implementation as cc_binary_with_flags.
+
+It is based off of:
+https://github.com/bazelbuild/examples/blob/7fc3f8b587ee415ff02ce358caa960f9533a912c/configurations/cc_test/defs.bzl
+
+"""
+
+load("//bazel:copts.bzl", "DEFAULT_COPTS")
+load("//bazel:cc_binary_with_flags.bzl", "with_flags_transition")
+
+def _transition_rule_impl(ctx):
+    executable_src = ctx.executable.actual_test
+    executable_dst = ctx.actions.declare_file(ctx.label.name)
+    ctx.actions.run_shell(
+        tools = [executable_src],
+        outputs = [executable_dst],
+        command = "cp %s %s" % (executable_src.path, executable_dst.path),
+    )
+    runfiles = ctx.attr.actual_test[0][DefaultInfo].default_runfiles
+    return [DefaultInfo(runfiles = runfiles, executable = executable_dst)]
+
+# This rule must end with a _test suffix or Bazel doesn't allow the test attribute to be true.
+transition_test = rule(
+    implementation = _transition_rule_impl,
+    attrs = {
+        # set_flags is a dictionary with the keys being the short-form of a flag name
+        # (e.g. the part that comes after the colon) and the value being a list of values
+        # that the flag should be set to, regardless of the relevant CLI flags.
+        # https://bazel.build/rules/lib/attr#string_list_dict
+        "set_flags": attr.string_list_dict(),
+        # This is the cc_test that should be made with the flags being set.
+        # Note specifically how it is modified using with_flags_transition, which
+        # ensures that the flags propagates down the graph. Must be executable
+        # so the _transition_rule_impl can use it as an executable.
+        # https://bazel.build/rules/lib/attr#label
+        "actual_test": attr.label(cfg = with_flags_transition, executable = True),
+        # This is a stock Bazel requirement for any rule that uses Starlark
+        # transitions. It's okay to copy the below verbatim for all such rules.
+        #
+        # The purpose of this requirement is to give the ability to restrict
+        # which packages can invoke these rules, since Starlark transitions
+        # make much larger graphs possible that can have memory and performance
+        # consequences for your build. The allowlist defaults to "everything".
+        # But you can redefine it more strictly if you feel that's prudent.
+        "_allowlist_function_transition": attr.label(
+            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
+        ),
+    },
+    # Means it works with "$ bazel test". https://bazel.build/rules/lib/globals#rule.test
+    test = True,
+)
+
+def cc_test_with_flags(name, set_flags = {}, copts = DEFAULT_COPTS, **kwargs):
+    """Builds a cc_test as if set_flags were set on the CLI.
+
+    Args:
+        name: string, the name for the rule that is the binary, but with the flags changed via
+            a transition. Any dependents should use this name.
+        set_flags: dictionary of string to list of strings. The keys should be the name of the
+            flag, and the values should be the desired valid settings for that flag.
+        copts: a list of strings or select statements that control the compiler flags.
+            It has a sensible list of defaults.
+        **kwargs: Any flags that a cc_binary normally takes.
+    """
+    cc_test_name = name + "_native_test"
+    transition_test(
+        name = name,
+        actual_test = ":%s" % cc_test_name,
+        set_flags = set_flags,
+        testonly = True,
+    )
+    tags = kwargs.get("tags", [])
+    tags.append("manual")  # We want to exclude this helper test from bazel test foo/...
+    kwargs["tags"] = tags
+    native.cc_test(
+        name = cc_test_name,
+        copts = copts,
+        **kwargs
+    )

bazel/common_config_settings/BUILD.bazel

@@ -1,4 +1,4 @@
-load("//bazel:macros.bzl", "bool_flag", "string_flag_with_values")
+load("//bazel:macros.bzl", "bool_flag", "selects", "string_flag_with_values")
 
 licenses(["notice"])
 
@@ -109,6 +109,7 @@ string_flag_with_values(
     name = "fontmgr_factory",
     default = "empty_fontmgr_factory",
     values = [
+        "android_fontmgr_factory",
         # Makes the default SkFontMgr load fonts from a hard-coded directory on disk.
         "custom_directory_fontmgr_factory",
         # Makes the default SkFontMgr load fonts from an SkEmbeddedResource that has been compiled
@@ -121,6 +122,8 @@ string_flag_with_values(
         # Makes the default SkFontMgr return null. Typically used when font support is not desired.
         "empty_fontmgr_factory",
         "fontconfig_fontmgr_factory",
+        # Deprecated, do not use.
+        "fci_fontmgr_factory",
     ],
 )
 
@@ -132,6 +135,7 @@ string_flag_with_values(
     name = "include_fontmgr",
     multiple = True,
     values = [
+        "android_fontmgr",
         # Allows the construction of an SkFontMgr that loads files from a programmatically
         # defined directory on disk.
         "custom_directory_fontmgr",
@@ -141,6 +145,8 @@ string_flag_with_values(
         # Allows the construction of an SkFontMgr which returns empty fonts.
         "custom_empty_fontmgr",
         "fontconfig_fontmgr",
+        # Deprecated, do not use.
+        "fci_fontmgr",
     ],
 )
 
@@ -164,3 +170,53 @@ bool_flag(
     name = "use_icu",
     default = False,
 )
+
+# These are some helpers to mean "either the fontmgr was enabled or its factory was"
+
+selects.config_setting_group(
+    name = "uses_android_fontmgr",
+    match_any = [
+        ":android_fontmgr",
+        ":android_fontmgr_factory",
+    ],
+)
+
+selects.config_setting_group(
+    name = "uses_custom_directory_fontmgr",
+    match_any = [
+        ":custom_directory_fontmgr",
+        ":custom_directory_fontmgr_factory",
+    ],
+)
+
+selects.config_setting_group(
+    name = "uses_custom_embedded_fontmgr",
+    match_any = [
+        ":custom_embedded_fontmgr",
+        ":custom_embedded_fontmgr_factory",
+    ],
+)
+
+selects.config_setting_group(
+    name = "uses_custom_empty_fontmgr",
+    match_any = [
+        ":custom_empty_fontmgr",
+        ":custom_empty_fontmgr_factory",
+    ],
+)
+
+selects.config_setting_group(
+    name = "uses_fontconfig_fontmgr",
+    match_any = [
+        ":fontconfig_fontmgr",
+        ":fontconfig_fontmgr_factory",
+    ],
+)
+
+selects.config_setting_group(
+    name = "uses_fci_fontmgr",
+    match_any = [
+        ":fci_fontmgr",
+        ":fci_fontmgr_factory",
+    ],
+)

include/ports/BUILD.bazel

@@ -1,28 +1,41 @@
-load("//bazel:macros.bzl", "exports_files_legacy")
+load("//bazel:macros.bzl", "exports_files_legacy", "select_multi")
 
 licenses(["notice"])
 
 exports_files_legacy()
 
+filegroup(
+    name = "fontmgr",
+    srcs = select_multi(
+        {
+            "//bazel/common_config_settings:uses_android_fontmgr": ["SkFontMgr_android.h"],
+            "//bazel/common_config_settings:uses_custom_directory_fontmgr": ["SkFontMgr_directory.h"],
+            "//bazel/common_config_settings:uses_custom_empty_fontmgr": ["SkFontMgr_empty.h"],
+            "//bazel/common_config_settings:uses_fontconfig_fontmgr": ["SkFontMgr_fontconfig.h"],
+            "//bazel/common_config_settings:uses_fci_fontmgr": [
+                "SkFontConfigInterface.h",
+                "SkFontMgr_FontConfigInterface.h",
+            ],
+            # TODO(kjlubick, bungeman) fuchsia_fontmgr, fontmgr_mac_ct, fontmgr_win
+        },
+    ),
+)
+
 filegroup(
     name = "public_hdrs",
     srcs = [
-        "SkCFObject.h",
-        "SkFontConfigInterface.h",
-        "SkFontMgr_FontConfigInterface.h",
-        "SkFontMgr_android.h",
-        "SkFontMgr_directory.h",
-        "SkFontMgr_empty.h",
-        "SkFontMgr_fontconfig.h",
-        "SkFontMgr_fuchsia.h",
-        "SkFontMgr_indirect.h",
-        "SkFontMgr_mac_ct.h",
         "SkImageGeneratorCG.h",
         "SkImageGeneratorNDK.h",
         "SkImageGeneratorWIC.h",
-        "SkRemotableFontMgr.h",
-        "SkTypeface_mac.h",
-        "SkTypeface_win.h",
-    ],
+        ":fontmgr",
+    ] + select({
+        "@platforms//os:macos": ["SkCFObject.h"],
+        "@platforms//os:ios": ["SkCFObject.h"],
+        "//conditions:default": [],
+    }) + select({
+        "@platforms//os:macos": ["SkTypeface_mac.h"],
+        "@platforms//os:windows": ["SkTypeface_win.h"],
+        "//conditions:default": [],
+    }),
     visibility = ["//include:__pkg__"],
 )