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

[Tests] Add TestNG listeners for resource cleanup, thread leak detection and "fail fast" #10195

Merged

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Apr 12, 2021

Motivation

There are a lot of resource cleanup issues in Pulsar tests. This PR adds some TestNG listeners to mitigate the problem.

Modifications

  • add TestNG listeners for cleaning up

    • Mockito mocking state in all threads
    • FastThreadLocal state holding org.apache.pulsar.* class instances in all threads
  • add TestNG listener for detecting leaked threads

    • This helps noticing and fixing issues caused by threads that are kept running after the test completes
  • rewrite RetryAnalyzer based on TestNG's RetryAnalyzerCount class

    • add tests for RetryAnalyzer
  • Fix test dependencies in pulsar-io/flume and pulsar-io/netty

  • add custom fail fast solution

    • Maven Surefire built-in solution is broken with TestNG 7.3.0
    • This makes the test retries in CI faster since most test runs are retries 3 times. It's a waste of resources to keep executing after the first test failure. (This applies at least for the 2 first runs out of the maximum of 3 runs. It could be useful to disable "fail fast" for the last attempt, however that is out-of-scope of this PR.)

- add listeners for cleaning up
  - Mockito mocking state in all threads
  - FastThreadLocal state with org.apache.pulsar.* class instances
    in all threads

- add listener for detecting leaked threads

- rewrite RetryAnalyzer based on TestNG's RetryAnalyzerCount class
  - add tests for RetryAnalyzer

- Fix test dependencies in pulsar-io/flume and pulsar-io/netty

- add custom fail fast solution
  - Maven Surefire built-in solution is broken with TestNG 7.3.0
@lhotari lhotari changed the title Add TestNG listeners for "fail fast" and resource cleanup [Tests] Add TestNG listeners for resource cleanup, thread leak detection and "fail fast" Apr 12, 2021
@lhotari
Copy link
Member Author

lhotari commented Apr 13, 2021

/pulsarbot run-failure-checks

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

@codelipenghui @congbobo184 PTAL
CI passed

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants