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

This is a reland of commit e034091

Original change's description:
> [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]>

Bug: skia:13758
Change-Id: Ib705dd1b6b411209628503b371b5c3b7ad24d2a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/588040
Reviewed-by: Leandro Lovisolo <[email protected]>
Reviewed-by: Ben Wagner <[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__"],
 )