Skip to content
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

AzureIntegrationTests share one billing profile #1702

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from

Commits on Jun 5, 2024

  1. AzureIntegrationTests share one billing profile

    I first converted AzureIntegrationTest to JUnit5.
    This was necessary in order to create @BeforeAll and @afterall methods to initialize a single Azure billing profile for use in all tests.
    
    Expected benefits:
    
    - If one test in the suite fails to delete its billing profile on tear-down, all other tests in the suite currently fail fast when trying to create datasets with errors like "Cannot reuse existing application deployment integrationapp from profile … with a different profile …".  This change should allow for more precise failure signaling, and increase the efficacy of retries.
    - Serve as a model for sharing test initialization within a class, not currently used elsewhere in TDR.  Overall integration test runtime can take 2+ hours, and sharing set-up may help drive that down while incentivizing writing more granular tests when appropriate rather than overloading existing ones.
    okotsopoulos committed Jun 5, 2024
    Configuration menu
    Copy the full SHA
    9fa7762 View commit details
    Browse the repository at this point in the history
  2. JUnit 5 supports Extensions, not Rules

    AzureIntegrationTest had a TestJobWatcher included as a @rule, which is no longer supported in JUnit 5.
    Instead, we can extend the test class with new TestJobWatcherExtension, which is compatible with JUnit 5.
    Also refactored two assertions which still used JUnit 4 methods.
    okotsopoulos committed Jun 5, 2024
    Configuration menu
    Copy the full SHA
    82963c2 View commit details
    Browse the repository at this point in the history
  3. Revert before merge: only run AzureIntegrationTest suite in CI

    To make debugging easier.
    okotsopoulos committed Jun 5, 2024
    Configuration menu
    Copy the full SHA
    53412c7 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    1e76844 View commit details
    Browse the repository at this point in the history
  5. Move blob IO test util initialization back to @beforeeach methods

    To help triangulate source of flaky integration tests.
    The valuable part of this work is sharing a billing profile among tests, so removing other possible sources of flakiness.
    okotsopoulos committed Jun 5, 2024
    Configuration menu
    Copy the full SHA
    cf19624 View commit details
    Browse the repository at this point in the history

Commits on Jun 10, 2024

  1. @pshapiro4broad PR feedback

    - Lost a closing quote when converting to multiline string
    - Typo in docstring
    okotsopoulos committed Jun 10, 2024
    Configuration menu
    Copy the full SHA
    b190687 View commit details
    Browse the repository at this point in the history
  2. Make UsersBase an injectable component

    Autowire it into AzureIntegrationTest rather than having the test extend it.
    Fix an instance where we mistakenly called UsersBase.steward() in a test where we should have used the steward user initialized in test setup.
    okotsopoulos committed Jun 10, 2024
    Configuration menu
    Copy the full SHA
    4860ebd View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    6acbeaf View commit details
    Browse the repository at this point in the history
  4. On Azure profile create, register association with deployed app

    Previously, the deployed app <-> billing profile association is registered the first time a dataset is created off of the profile.
    I believe this is producing a race condition / flaky integration tests using the same billing profile.
    
    - Added new CreateProfileRegisterDeployedApplicationStep in ProfileCreateFlight
    - Notably, added this after the profile metadata was saved and made available as a job response in the working map
    - Expanded docstrings, unit testing of AzureApplicationDeploymentService.getOrRegisterApplicationDeployment
    okotsopoulos committed Jun 10, 2024
    Configuration menu
    Copy the full SHA
    6c1199f View commit details
    Browse the repository at this point in the history

Commits on Jun 11, 2024

  1. Debug failing tests: disable AzureAuthService caching

    Test failures and flakiness seems to happen…
    - During dataset creation…
    - Specifically in CreateDatasetGetOrCreateStorageAccountStep when getting an existing SA (a new condition when billing profiles are shared, previously a new one was created with each test case)…
    - Specifically in the call to azureBlobStorePdao.enableFileLogging
    okotsopoulos committed Jun 11, 2024
    Configuration menu
    Copy the full SHA
    8d659c4 View commit details
    Browse the repository at this point in the history

Commits on Jun 12, 2024

  1. Explicitly null out resource identifiers on set-up

    Otherwise, test tear-down for a different test may unsuccessfully try to delete them -- observed for releaseSnapshotId specifically, which is not initialized in all tests.
    okotsopoulos committed Jun 12, 2024
    Configuration menu
    Copy the full SHA
    ad4aece View commit details
    Browse the repository at this point in the history