Skip to content

Conversation

@aaron-steinfeld
Copy link
Contributor

Description

Fixed a few issues that came up when debugging an integration test shutdown. Shutdown was called an excessive number of times:

  1. We called IntegrationTestServiceLauncher.shutdown() for every running service, but shutdown() already loops through all services.
  2. We had a shutdown hook that ran on thread close, but the tests are manually shutdown (and thread is shared across tests so doesn't match the test's lifecycle
  3. We used a static PLATFORM_SERVICES variable to track started services, then shutdown loops through each and shuts them down. This static was never cleared, so the same services were accumulating across tests.

@aaron-steinfeld aaron-steinfeld requested a review from a team as a code owner June 28, 2023 06:18
@github-actions
Copy link

github-actions bot commented Jun 28, 2023

Test Results

31 tests  ±0   31 ✔️ ±0   11s ⏱️ ±0s
  9 suites ±0     0 💤 ±0 
  9 files   ±0     0 ±0 

Results for commit 97d62e0. ± Comparison against base commit d518316.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Jun 28, 2023

Codecov Report

Merging #72 (97d62e0) into main (d518316) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main      #72   +/-   ##
=========================================
  Coverage     70.57%   70.57%           
  Complexity      106      106           
=========================================
  Files            15       15           
  Lines           588      588           
  Branches         32       32           
=========================================
  Hits            415      415           
  Misses          154      154           
  Partials         19       19           
Flag Coverage Δ
unit 70.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

kotharironak
kotharironak previously approved these changes Jun 28, 2023
id("org.hypertrace.jacoco-report-plugin") version "0.2.0" apply false
id("org.hypertrace.code-style-plugin") version "1.1.2" apply false
id("org.owasp.dependencycheck") version "8.2.1"
id("org.owasp.dependencycheck") version "8.3.1"
Copy link
Contributor Author

@aaron-steinfeld aaron-steinfeld Jun 28, 2023

Choose a reason for hiding this comment

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

We'll need to be careful with this one. It introduces a subtle issue that requires a gradle version upgrade.

@aaron-steinfeld aaron-steinfeld merged commit 4e0c1b4 into main Jun 29, 2023
@aaron-steinfeld aaron-steinfeld deleted the fix-itest-cleanup branch June 29, 2023 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants