Skip to content

[improve][ci] Improve TestNG test execution to debug flaky test failures#20425

Merged
tisonkun merged 1 commit intoapache:masterfrom
lhotari:lh-fix-cleanup-with-failfast
May 29, 2023
Merged

[improve][ci] Improve TestNG test execution to debug flaky test failures#20425
tisonkun merged 1 commit intoapache:masterfrom
lhotari:lh-fix-cleanup-with-failfast

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented May 29, 2023

Motivation

In order to investigate some flaky test failures such as #20386, it is necessary to add logging to find out what was the failure of the first attempt. In some cases, the exception in the test retry is not the root cause.

In the case of #20386, there's "Index 0 out of bounds for length 0" recorded in the skip message:

  <testcase name="testMsgDropStat" classname="org.apache.pulsar.client.api.NonPersistentTopicTest" time="0">
    <skipped message="Index 0 out of bounds for length 0"/>

However, the full exception isn't logged.

Modifications

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

- Log possible exceptions before retrying a test run
  - this will help debug issues when tests are retried
- Don't log anything when skipping is a result of fail fast mode (SkipException)
- Execute cleanup methods when using fail fast mode
 - the change in apache#19252 broke the logic that intends to run cleanup methods after the first failure
@codecov-commenter
Copy link

Codecov Report

Merging #20425 (d730f75) into master (aa3bfcd) will decrease coverage by 0.01%.
The diff coverage is 78.57%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #20425      +/-   ##
============================================
- Coverage     72.90%   72.90%   -0.01%     
- Complexity    31864    31877      +13     
============================================
  Files          1864     1865       +1     
  Lines        138416   138439      +23     
  Branches      15188    15192       +4     
============================================
+ Hits         100919   100932      +13     
- Misses        29477    29491      +14     
+ Partials       8020     8016       -4     
Flag Coverage Δ
inttests 24.20% <10.71%> (+0.02%) ⬆️
systests 25.00% <10.71%> (-0.07%) ⬇️
unittests 72.18% <78.57%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...a/org/apache/bookkeeper/mledger/ManagedCursor.java 42.85% <0.00%> (-3.30%) ⬇️
...a/org/apache/bookkeeper/mledger/ManagedLedger.java 0.00% <0.00%> (ø)
...rg/apache/bookkeeper/mledger/impl/OpReadEntry.java 84.31% <75.00%> (+4.72%) ⬆️
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java 78.79% <76.92%> (+0.33%) ⬆️
...che/bookkeeper/mledger/impl/ManagedLedgerImpl.java 81.32% <100.00%> (+0.16%) ⬆️
...ookkeeper/mledger/impl/ManagedLedgerMBeanImpl.java 89.31% <100.00%> (ø)
...kkeeper/mledger/impl/cache/EntryCacheDisabled.java 70.83% <100.00%> (ø)
...keeper/mledger/impl/cache/RangeEntryCacheImpl.java 58.75% <100.00%> (ø)

... and 65 files with indirect coverage changes

@tisonkun tisonkun merged commit 0bdcaec into apache:master May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test type/flaky-tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants