starboard/nplb: Fold nplb_evergreen_compat_tests into nplb proper by yell0wd0g · Pull Request #4946 · youtube/cobalt

yell0wd0g · 2025-02-21T05:39:10Z

This is a follow-up to #4831, rolling nplb_evergreen_compat_tests into nplb proper.

Run via ./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest* --

Two sets of test failed on my Linux and needed attention:

CrashpadConfigTest.VerifyUploadCert was IIUC broken due to an
extra ...app/cobalt/content... path manually added to the cert
location. Looking at the GN ssl:copy_ssl_certificates target, it
seems to agree with GetCACertificatesPath() in that the extra path
is not needed. Discussing this with @y4vor, we decided to add a new
helper target copy_ssl_certificates_evergreen to put the certs
in the right spot.
FontsTest.* needed deps on fonts:copy_font_data GN targets
(both data_deps and deps). This worked, but I had to comment
out the install_content = true statements in that BUILD.gn (it's
unclear whether this would break other things). Said GN target
uses exec_script and needs an exception in //.gn, this CL adds
that too.

Asides from it, some unnecessary test classes are removed, and some
preprocessor logic simplified.

Bug: b/384819454

andrewsavage1

I think this is reasonable and LGTM but I'll leave it to Holden who knows more about these tests

hlwarriner · 2025-02-21T16:44:02Z

These tests are tricky and I think it unfortunately would be more trouble than it's worth to move them into nplb at this time.

They were never designed to run in CI and there are several reasons why they can't currently be expected to pass in CI across our reference devices: https://b.corp.google.com/issues/292138589#comment2. We could add filters for the problematic tests, but we'd probably end up filtering over half of them.

We may end up simply removing these tests soon. They were originally intended as a quick way for partners to assess progress in their Evergreen integrations, and their test coverage is very minimal compared to the Evergreen YTS tests. I need to revisit b/292138589 and make a proposal, but I think what we'll end up doing is retiring these tests and adding any extra coverage they provide to YTS.

Also, if we we do want to land this PR then we'll need to update a few partner docs (e.g., https://github.com/youtube/cobalt/blob/main/starboard/doc/evergreen/cobalt_evergreen_overview.md, https://developers.google.com/youtube/cobalt/docs/development/setup-android).

@yuying-y @lamdbui in case you disagree with anything here.

yell0wd0g · 2025-02-21T17:50:34Z

We may end up simply removing these tests soon.

I like this part. If these tests are nplb, then they should run there. Otherwise it's hard to justify their existence; the point of nplb is to be a single integration test.

yell0wd0g · 2025-02-21T21:11:07Z

@hlwarriner and I talked about this. Next PR proposal is:

Build all of these tests when sb_is_evergreen_compatible (this CL asserts for it in the files instead of using it to sandwich the contents) as part of nplb.
Some of the actual tests fail on e.g. Linux; this is because sb_is_evergreen_compatible doesn't mean sb_is_evergreen -- so this CL adds GTEST_SKIP() in those cases. (These test IIUC should also be skipped on e.g. Raspi, but not on e.g. RDK which is a fully fledged EG board).

@y4vor FYI

hlwarriner · 2025-02-21T21:50:58Z

@hlwarriner and I talked about this. Next PR proposal is:

Build all of these tests when sb_is_evergreen_compatible (this CL asserts for it in the files instead of using it to sandwich the contents) as part of nplb.

Some of the actual tests fail on e.g. Linux; this is because sb_is_evergreen_compatible doesn't mean sb_is_evergreen -- so this CL adds GTEST_SKIP() in those cases. (These test IIUC should also be skipped on e.g. Raspi, but not on e.g. RDK which is a fully fledged EG board).

@y4vor FYI

Just want to clarify something about sb_is_evergreen_compatible and sb_is_evergreen.

sb_is_evergreen_compatible should only be set for "loader platforms" that can build the elf_loader and loader_app which, when run, can load the evergreen shared library (provided by a different platform). An evergreen partner building their partner-built components would want to set this in their port.
sb_is_evergreen should only be set for the "evergreen platform" that is used to build the evergreen shared library. Partners should never set this, since these platforms are maintained by google (for a handful of architectures) and libcobalt.so is built by google.

What's kind of missing, and is highlighted by issue 2 from https://b.corp.google.com/issues/292138589#comment2, is something to capture whether an sb_is_evergreen_compatible platform is actively being used as an evergreen loader platform in the current build. In other words, linux-x64x11 is always evergreen-compatible but it isn't always used as an evergreen loader platform...sometimes it's just used to build a cobalt executable and to run plain, non-evergreen cobalt.

yell0wd0g · 2025-02-21T21:57:11Z

@hlwarriner and I talked about this. Next PR proposal is:

Build all of these tests when sb_is_evergreen_compatible (this CL asserts for it in the files instead of using it to sandwich the contents) as part of nplb.

Some of the actual tests fail on e.g. Linux; this is because sb_is_evergreen_compatible doesn't mean sb_is_evergreen -- so this CL adds GTEST_SKIP() in those cases. (These test IIUC should also be skipped on e.g. Raspi, but not on e.g. RDK which is a fully fledged EG board).

@y4vor FYI

Just want to clarify something about sb_is_evergreen_compatible and sb_is_evergreen.

sb_is_evergreen_compatible should only be set for "loader platforms" that can build the elf_loader and loader_app which, when run, can load the evergreen shared library (provided by a different platform). An evergreen partner building their partner-built components would want to set this in their port.

sb_is_evergreen should only be set for the "evergreen platform" that is used to build the evergreen shared library. Partners should never set this, since these platforms are maintained by google (for a handful of architectures) and libcobalt.so is built by google.

What's kind of missing, and is highlighted by issue 2 from b.corp.google.com/issues/292138589#comment2, is something to capture whether an sb_is_evergreen_compatible platform is actively being used as an evergreen loader platform in the current build. In other words, linux-x64x11 is always evergreen-compatible but it isn't always used as an evergreen loader platform...sometimes it's just used to build a cobalt executable and to run plain, non-evergreen cobalt.

To further the point of this CL, and perhaps erroneously, this is my understanding:

	sb_is_evergreen_compatible	sb_is_evergreen
Linux	yes	no
raspi	yes	No?
rdk	yes	yes!
aosp	yes	yes?
Android TV	No (!)	No
partner TVs (Samsung etc)	yes ofc	yes!

Ofc sb_is_evergreen -> sb_is_evergreen_compatible (with -> being a logical if-then, "implies").

hlwarriner · 2025-02-21T22:09:12Z

@hlwarriner and I talked about this. Next PR proposal is:

Build all of these tests when sb_is_evergreen_compatible (this CL asserts for it in the files instead of using it to sandwich the contents) as part of nplb.

Some of the actual tests fail on e.g. Linux; this is because sb_is_evergreen_compatible doesn't mean sb_is_evergreen -- so this CL adds GTEST_SKIP() in those cases. (These test IIUC should also be skipped on e.g. Raspi, but not on e.g. RDK which is a fully fledged EG board).

@y4vor FYI

Just want to clarify something about sb_is_evergreen_compatible and sb_is_evergreen.

sb_is_evergreen_compatible should only be set for "loader platforms" that can build the elf_loader and loader_app which, when run, can load the evergreen shared library (provided by a different platform). An evergreen partner building their partner-built components would want to set this in their port.

sb_is_evergreen should only be set for the "evergreen platform" that is used to build the evergreen shared library. Partners should never set this, since these platforms are maintained by google (for a handful of architectures) and libcobalt.so is built by google.

What's kind of missing, and is highlighted by issue 2 from b.corp.google.com/issues/292138589#comment2, is something to capture whether an sb_is_evergreen_compatible platform is actively being used as an evergreen loader platform in the current build. In other words, linux-x64x11 is always evergreen-compatible but it isn't always used as an evergreen loader platform...sometimes it's just used to build a cobalt executable and to run plain, non-evergreen cobalt.

To further the point of this CL, and perhaps erroneously, this is my understanding:

sb_is_evergreen_compatible sb_is_evergreen
Linux yes no
raspi yes No?
rdk yes yes!
aosp yes yes?
Android TV No (!) No
partner TVs (Samsung etc) yes ofc yes!
Ofc sb_is_evergreen -> sb_is_evergreen_compatible (with -> being a logical if-then, "implies").

No, I don't think that's quite right. sb_is_evergreen doesn't imply sb_is_evergreen_compatible, they are actually mutually exclusive.

A couple examples from 25.lts.1+ of evergreen-compatible platform and evergreen platform pairs, from our reference ports:

Evergreen-compatible platform //starboard/linux/x64x11 can be used with Evergreen platform //starboard/evergreen/x64
Evergreen-compatible platform //starboard/raspi/2 can be used with Evergreen platform //starboard/evergreen/arm/hardfp

y4vor · 2025-02-24T20:31:57Z

@hlwarriner and I talked about this. Next PR proposal is:

Build all of these tests when sb_is_evergreen_compatible (this CL asserts for it in the files instead of using it to sandwich the contents) as part of nplb.

Some of the actual tests fail on e.g. Linux; this is because sb_is_evergreen_compatible doesn't mean sb_is_evergreen -- so this CL adds GTEST_SKIP() in those cases. (These test IIUC should also be skipped on e.g. Raspi, but not on e.g. RDK which is a fully fledged EG board).

@y4vor FYI

Just want to clarify something about sb_is_evergreen_compatible and sb_is_evergreen.

sb_is_evergreen_compatible should only be set for "loader platforms" that can build the elf_loader and loader_app which, when run, can load the evergreen shared library (provided by a different platform). An evergreen partner building their partner-built components would want to set this in their port.

sb_is_evergreen should only be set for the "evergreen platform" that is used to build the evergreen shared library. Partners should never set this, since these platforms are maintained by google (for a handful of architectures) and libcobalt.so is built by google.

What's kind of missing, and is highlighted by issue 2 from b.corp.google.com/issues/292138589#comment2, is something to capture whether an sb_is_evergreen_compatible platform is actively being used as an evergreen loader platform in the current build. In other words, linux-x64x11 is always evergreen-compatible but it isn't always used as an evergreen loader platform...sometimes it's just used to build a cobalt executable and to run plain, non-evergreen cobalt.

To further the point of this CL, and perhaps erroneously, this is my understanding:
sb_is_evergreen_compatible sb_is_evergreen
Linux yes no
raspi yes No?
rdk yes yes!
aosp yes yes?
Android TV No (!) No
partner TVs (Samsung etc) yes ofc yes!
Ofc sb_is_evergreen -> sb_is_evergreen_compatible (with -> being a logical if-then, "implies").

No, I don't think that's quite right. sb_is_evergreen doesn't imply sb_is_evergreen_compatible, they are actually mutually exclusive.

A couple examples from 25.lts.1+ of evergreen-compatible platform and evergreen platform pairs, from our reference ports:

Evergreen-compatible platform //starboard/linux/x64x11 can be used with Evergreen platform //starboard/evergreen/x64

Evergreen-compatible platform //starboard/raspi/2 can be used with Evergreen platform //starboard/evergreen/arm/hardfp

The meaning of these flags is the following:
sb_is_evergreen: Applies only above Starboard and means it is an Evergreen/Hermetic build.
sb_is_evergreen_compatible: Applies only at the Starboard implementation and means the build would be used for Evergreen.

yell0wd0g · 2025-02-25T00:27:24Z

The meaning of these flags is the following:
sb_is_evergreen: Applies only above Starboard and means it is an Evergreen/Hermetic build.
sb_is_evergreen_compatible: Applies only at the Starboard implementation and means the build would be used for Evergreen.

Gotcha thanks for the explanation. I've changed the code to be more specific: SKIP those 3 test cases on platforms where we don't have the paths. Currently this is only Linux, since there it doesn't make sense to have to create e.g. kSbSystemPathFontDirectory just for passing an integration test. PTAL

hlwarriner

Left one comment requesting a small doc change but otherwise lgtm.

Let's just wait for approval from @y4vor , too.

…on-EVERGREEN (full) platforms

…een_compat_tests that don't apply.

- CrashpadConfigTest.VerifyUploadCert was IIUC broken due to an extra ...app/cobalt/content... path manually added to the cert location. Looking at the GN ssl:copy_ssl_certificates target, it seems to agree with GetCACertificatesPath(). The test logic is fixed and extended. The class is unnecessary, so it's removed. - FontsTest.* needed deps on fonts:copy_font_data GN targets (both data_deps and deps). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (unclear whether this would break other things). Said GN target uses exec_script and needs an exception in //.gn.

…ing copy_ssl_certificates_evergreen target. Removed unnecessary Gtest classes

yell0wd0g

@y4vor PTAL

@andrewsavage1 I had to do something fishy in starboard/content/fonts/BUILD.gn -- would you mind taking a look..? Thanks!

…after chatting with Andrew

…content/ssl/BUILD.gn

…tests__to_nplb

@y4vor

…utube#4946) This is a follow-up to youtube#4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454

@y4vor

) This is a follow-up to #4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

) This is a follow-up to #4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

Original Message: starboard/nplb: Fold nplb_evergreen_compat_tests into nplb proper (youtube#4946) This is a follow-up to youtube#4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112 Files with conflicts: - starboard/content/fonts/BUILD.gn - starboard/doc/evergreen/cobalt_evergreen_overview.md - starboard/nplb/nplb_evergreen_compat_tests/crashpad_config_test.cc - starboard/nplb/nplb_evergreen_compat_tests/executable_memory_test.cc - starboard/nplb/nplb_evergreen_compat_tests/fonts_test.cc - starboard/nplb/nplb_evergreen_compat_tests/loader_app_metrics_test.cc - starboard/nplb/nplb_evergreen_compat_tests/max_file_name_test.cc - starboard/nplb/nplb_evergreen_compat_tests/sabi_test.cc - starboard/nplb/nplb_evergreen_compat_tests/storage_test.cc - third_party/tflite_support/src/tensorflow_lite_support/custom_ops/kernel/_pywrap_ngrams_op_resolver.pyi

@y4vor

…utube#4946) This is a follow-up to youtube#4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

…utube#4946) This is a follow-up to youtube#4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

…utube#4946) This is a follow-up to youtube#4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

) This is a follow-up to #4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

) This is a follow-up to #4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

) This is a follow-up to #4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

…utube#4946) This is a follow-up to youtube#4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

@y4vor

…utube#4946) This is a follow-up to youtube#4831, rolling `nplb_evergreen_compat_tests` into `nplb` proper. Run via `./out/linux-x64x11_devel/nplb --gtest_filter=*CrashpadConfigTest*:*SabiTest*:*ExecutableMemoryTest*:*MaxFileNameTest*:*StorageTest*:*LoaderAppMetricsTest:*FontsTest*` -- Two sets of test failed on my Linux and needed attention: - `CrashpadConfigTest.VerifyUploadCert` was IIUC broken due to an extra `...app/cobalt/content...` path manually added to the cert location. Looking at the GN `ssl:copy_ssl_certificates` target, it seems to agree with `GetCACertificatesPath()` in that the extra path is not needed. Discussing this with @y4vor, we decided to add a new helper target `copy_ssl_certificates_evergreen` to put the certs in the right spot. - `FontsTest.*` needed `deps` on `fonts:copy_font_data` GN targets (both `data_deps` and `deps`). This worked, but I had to comment out the `install_content = true` statements in that BUILD.gn (it's unclear whether this would break other things). Said GN target uses `exec_script` and needs an exception in //.gn, this CL adds that too. Asides from it, some unnecessary test classes are removed, and some preprocessor logic simplified. Bug: b/384819454 original-hexsha: 6e8f112

yell0wd0g requested review from andrewsavage1 and hlwarriner February 21, 2025 05:39

yell0wd0g mentioned this pull request Feb 21, 2025

starboard: Create starboard_unittest and fold in a few test targets #4831

Merged

andrewsavage1 reviewed Feb 21, 2025

View reviewed changes

hlwarriner requested review from lamdbui and yuying-y February 21, 2025 16:44

yell0wd0g force-pushed the b_384819454__sidegig__nplb_evergreen_compat_tests__to_nplb branch from 07e7af5 to 181395e Compare February 21, 2025 21:06

yell0wd0g commented Feb 22, 2025

View reviewed changes

Comment thread starboard/nplb/nplb_evergreen_compat_tests/fonts_test.cc Resolved

yell0wd0g force-pushed the b_384819454__sidegig__nplb_evergreen_compat_tests__to_nplb branch from c19fee2 to 724a860 Compare February 25, 2025 00:27

yell0wd0g force-pushed the b_384819454__sidegig__nplb_evergreen_compat_tests__to_nplb branch from 724a860 to 5343c27 Compare February 26, 2025 06:06

yell0wd0g requested a review from a team as a code owner February 26, 2025 06:06

yell0wd0g requested a review from dahlstrom-g February 26, 2025 06:06

hlwarriner reviewed Feb 26, 2025

View reviewed changes

Comment thread starboard/nplb/nplb_evergreen_compat_tests/crashpad_config_test.cc Resolved

Comment thread starboard/doc/evergreen/cobalt_evergreen_overview.md Resolved

hlwarriner approved these changes Feb 26, 2025

View reviewed changes

Comment thread starboard/doc/evergreen/cobalt_evergreen_overview.md Resolved

yell0wd0g added 7 commits February 26, 2025 20:24

Roll nplb_evergreen_compat_tests into nplb proper

2767183

Assert for EVERGREEN_COMPATIBLE in test files. GTEST_SKIP() some on n…

645b0e2

…on-EVERGREEN (full) platforms

Use special BUILD bricolage/preprocessor variable to SKIP nplb_evergr…

e265840

…een_compat_tests that don't apply.

Formatted //starboard/nplb/BUILD.gn as per presubmit

e7873a2

moarrrrr style

44a8736

Restore CrashpadConfigTest.VerifyUploadCert logic after adding and us…

960a0d4

…ing copy_ssl_certificates_evergreen target. Removed unnecessary Gtest classes

yell0wd0g force-pushed the b_384819454__sidegig__nplb_evergreen_compat_tests__to_nplb branch from 0da60e0 to 960a0d4 Compare February 27, 2025 04:34

yell0wd0g commented Feb 27, 2025

View reviewed changes

Comment thread starboard/doc/evergreen/cobalt_evergreen_overview.md Resolved

yell0wd0g added 2 commits February 26, 2025 20:42

MOARRRR Style

a87fde0

MOARRRR Style -- now also spell checked

c5482de

y4vor approved these changes Feb 27, 2025

View reviewed changes

Remove commented-out statement in //starboard/content/fonts/BUILD.gn …

6dd2f5f

…after chatting with Andrew

andrewsavage1 approved these changes Mar 3, 2025

View reviewed changes

Comment thread starboard/content/fonts/BUILD.gn Resolved

Comment thread starboard/content/ssl/BUILD.gn Resolved

Remove 'if (bla) { install_content = true }' statements in starboard/…

a07f133

…content/ssl/BUILD.gn

yell0wd0g force-pushed the b_384819454__sidegig__nplb_evergreen_compat_tests__to_nplb branch from 1e5d5f6 to a07f133 Compare March 3, 2025 18:58

Merge branch 'main' into b_384819454__sidegig__nplb_evergreen_compat_…

d52b259

…tests__to_nplb

yell0wd0g merged commit 6e8f112 into youtube:main Mar 3, 2025

hlwarriner mentioned this pull request May 8, 2025

Build nplb_evergreen_compat_tests with default toolchain #5691

Merged

Conversation

yell0wd0g commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewsavage1 left a comment

Choose a reason for hiding this comment

Uh oh!

hlwarriner commented Feb 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yell0wd0g commented Feb 21, 2025

Uh oh!

yell0wd0g commented Feb 21, 2025

Uh oh!

hlwarriner commented Feb 21, 2025

Uh oh!

yell0wd0g commented Feb 21, 2025

Uh oh!

hlwarriner commented Feb 21, 2025

Uh oh!

Uh oh!

y4vor commented Feb 24, 2025

Uh oh!

yell0wd0g commented Feb 25, 2025

Uh oh!

Uh oh!

Uh oh!

hlwarriner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yell0wd0g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yell0wd0g commented Feb 21, 2025 •

edited

Loading

hlwarriner commented Feb 21, 2025 •

edited

Loading