[RunAllTests] Consolidate testing to ensure all tests are covered in Bazel CI #2697
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduces top-level module tests so that all tests can be included by default. Adds a bit of extra redundant processing for app module tests. Also, fixes OptionsFragmentTest which wasn't previously included. Removes unnecessary DataBinderMapperImpl per fixing #1683. This results in all tests building, but they have not yet been confirmed to be passing.
Entailed: 1. Removing non-test targets from match 2. Readding databinder mapper file (not sure why it can't be removed, but not digging into it now) 3. Fix ExceptionsControllerTest so that it works correctly when run outside of Gradle 4. Other various fixes Altogether, 17 new tests were added to the Bazel test matrix.
BenHenning
changed the title
|
Forcing all tests to run in this PR to illustrate the new tests being covered. I can't rely on compute_affected_targets since there seem to be some issues with it at the moment (see #2691 (comment)). |
rt4914
approved these changes
Feb 12, 2021
Remove extra space.
fsharpasharp
approved these changes
Feb 15, 2021
…re-covered Conflicts: app/BUILD.bazel app/app_test.bzl domain/domain_test.bzl testing/BUILD.bazel testing/testing_test.bzl utility/BUILD.bazel utility/utility_test.bzl
Normalize Bazel docstrings.
|
Updated the PR to include some reworking of Bazel doc comments to be more normalized/consistent. |
vinitamurthi
approved these changes
Feb 17, 2021
|
@anandwana001 PTAL--we should try to get this in soon since it's reintroducing tests that were previously missed in Bazel builds. |
anandwana001
approved these changes
Feb 23, 2021
BenHenning
added
the
PR don't merge - NEEDS UPDATE
BenHenning
removed
the
PR don't merge - NEEDS UPDATE
BenHenning
deleted the
consolidate-testing-to-ensure-all-tests-are-covered
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates our test declarations such that all tests under each module are always included. In all, this effort has introduced 17 test targets that were previously missing.
Note that this is intentionally a stop-gap solution since we're planning to move all tests over to individual targets (at which point we will need another check in place to ensure tests are being properly added). To prepare for this work, each module BUILD file now has a MIGRATED_TESTS list that will automatically filter out migrated tests from module-level test generation.
This PR includes fixes for ExceptionsControllerTest (similar to the fixes for LogWorkerUploadTest in #1904) since stack traces differ at runtime between Gradle & Bazel. Also, this PR forces all app module tests to be resource-compatible which introduces an extra genrule for each one. This does affect the build time slightly, but it's not substantially noticeable unless large numbers of tests are being built/run at one time.