Skip to content

test: introduce new StorageITRunner #1785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Nov 30, 2022
Merged

test: introduce new StorageITRunner #1785

merged 28 commits into from
Nov 30, 2022

Conversation

BenWhitehead
Copy link
Collaborator

@BenWhitehead BenWhitehead commented Nov 22, 2022

A note to the reviewer:

This PR refactors the hole integration test suite and touches many files. However, the
majority of the files touched are not logically changing, simply changing how they declare
their test environment dependencies.

This PR is intentionally several commits, each commit is a small logical grouping of a
specific change. Take each commit in turn rather than the hole PR as a single change.

First read the rest of this description, then take a look at each individual commit.

The later majority of commits are simply migrating an individual test class to using
the new runners.

New StorageITRunner

Add two new JUnit runner to provide support for Cross Running test in a class across multiple
backends and transports.

When a class is annotated @CrossRun(transports = {HTTP, GRPC}, backends = {PROD, TEST_BENCH})
each test defined in the class will be ran four times, once for each pair from the permutation.
Each permutation will add a new entry to the test suite of the form [Transport][Backend].
This name form can be thought of as keys to access a location in an adjacency matrix.

If only a single backed is needed @SingleBackend() can be used to denote the backend.

If a class is annotated @ParallelFriendly the test suite may attempt to run the tests in
parallel. Currently parallelism factor is set to 2 * core count.

Field @Injection

When defining the test several "globally scoped" items are available for injection to the test
class by annotating a public non-final field with @Inject.

Instances of Storage, along with BucketInfo, TestBench, etc. are registered as injectable values.
If a field isn't able to be injected an InitializationError will be raised with details as to why.

CrossRun.Ignore & CrossRun.Exclude

Not all tests in a class will apply to all intersections, to allow filtering out tests for
these cases there are two new annotations which can be used. If a test is both Ignored and
Excluded, the Ignore will take precedence.

@Ignored functions similar to the standard @org.junit.Ignore where the test will be loaded
but not ran.

@Excluded tests will not be loaded into the suite at all, thereby not being reported at all.

@Parameterized running

When a class is annotated @Parameterized(ParameterProvider) in addition to and Cross Run
previously resolved, each parameter will result in another dimension added to the suite.
The name of the suite will be extended with another dimension below any cross name,
for example [HTTP][PROD][myParam].

Runner Registry

A new class Registry has been added which is responsible for managing "globally scoped"
instances of resources associated with running integration tests. All injectable instances
are registered with the Registry and will be lazily initialized, reused and cleaned up.

Instances like Storage clients, Backend Buckets, standard objects etc are all managed per
backend.

When a BucketInfo is loaded into the registry it's name is also made available to the
provider for Storage instances. If a test attempts to mutate the state of a bucket the
operation will be interrupted with a vetoing exception causing the test to fail.

Refactoring Done

  1. TestBench is now a Registry Resource and shouldn't be used outside that context. Since
    testbench binds ports it is not a multi instance thing anyway.
  2. Remove StorageFixture in favor of Registry Field Injection
  3. Remove BucketFixture in favor of Registry Field Injection
  4. All IT test now use either of the two new runners
  5. Remove ParallelParameterized runner, instead annotate your class with @ParallelFriendly
  6. Move bucket cleanup utility from BucketFixture to new class BucketCleaner

TODO

Items to still perform in later PRs

  • Add to contributing guide
  • Make TestBench no longer be a JUnit Rule - it should only be injected by our runner
  • Flush out StorageInstance proxy to prevent mutation of global buckets
  • Make each test hermetic to allow it running in parallel
  • Cleanup some minor TODO's in the Registry
  • Validate class annotations such that if @BeforeClass/@AfterClass/@ClassRule are used
    an InitializationError is raised stating the fact they are unsafe and will be ran
    multiple times (once for each sub-suite generated when cross running).
    • Provide acknowledge and processed annotation to allow a test to use the above
      annotations at their own discretion
  • Figure out why when running a single test the suite hierarchy shows up weird in intellij
    • The correct suite runs, simply the rendering is odd

Split enclosed classes into their own respective classes ITBlobReadMaskTest and ITBucketReadMaskTest
Refactor RetryTestFixture to extend TestWatcher to allow for manual registration
@BenWhitehead BenWhitehead requested review from a team as code owners November 22, 2022 18:46
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: storage Issues related to the googleapis/java-storage API. labels Nov 22, 2022
@BenWhitehead BenWhitehead changed the title test: introduce new StorageITRunner and StorageITParamRunner test: introduce new StorageITRunner Nov 30, 2022
Copy link
Contributor

@frankyn frankyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the discussion, PR LGTM, I added an item to add documentation to the contributing guide for using this infrastructure / linking to javadocs.

@BenWhitehead BenWhitehead merged commit 4aa5355 into main Nov 30, 2022
@BenWhitehead BenWhitehead deleted the custom-it-runner branch November 30, 2022 21:58
gcf-owl-bot bot added a commit that referenced this pull request Mar 31, 2023
Source-Link: googleapis/synthtool@43c709a
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:3387f93d4577788512112ff69ddab746ae9192ddd9f13cfd175ef310d62d7d30
BenWhitehead pushed a commit that referenced this pull request Mar 31, 2023
Source-Link: googleapis/synthtool@43c709a
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:3387f93d4577788512112ff69ddab746ae9192ddd9f13cfd175ef310d62d7d30

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants