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

ci(gha): check for duplicate test runs #10389

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

megglos
Copy link
Contributor

@megglos megglos commented Sep 16, 2022

Description

Adds a GHA that will check for any duplicate test runs from the maven build output of all maven test jobs, except the platform smoke tests.

Related issues

closes #10260

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/1.3) to the PR, in case that fails you need to create backports manually.

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually
  • The change has been verified by a QA run
  • The impact of the changes is verified by a benchmark

Documentation:

  • The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
  • New content is added to the release announcement
  • If the PR changes how BPMN processes are validated (e.g. support new BPMN element) then the Camunda modeling team should be informed to adjust the BPMN linting.

Please refer to our review guidelines.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 16, 2022

Test Results

   859 files  ±    0     859 suites  ±0   1h 51m 51s ⏱️ + 2m 40s
6 691 tests +173  6 680 ✔️ +173  11 💤 ±0  0 ±0 
6 875 runs  +173  6 864 ✔️ +173  11 💤 ±0  0 ±0 

Results for commit d2c4c41. ± Comparison against base commit 70bc392.

♻️ This comment has been updated with latest results.

@megglos megglos force-pushed the meg-10260-gha-duplicate-test-check branch 10 times, most recently from 759b304 to 8217dfd Compare September 16, 2022 20:48
@megglos
Copy link
Contributor Author

megglos commented Sep 16, 2022

Provoked duplicated test runs with a temporary commit here:
https://github.com/camunda/zeebe/actions/runs/3070648153/jobs/4960585093

@megglos megglos force-pushed the meg-10260-gha-duplicate-test-check branch from 8217dfd to d8836e3 Compare September 16, 2022 21:23
@megglos megglos marked this pull request as ready for review September 16, 2022 21:23
Copy link
Member

@lenaschoenburg lenaschoenburg left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me 👍

This has the drawback that we are not checking for duplicate runs across jobs but I think the original motivation was to prevent overriding test reports so that should be fine.

@megglos
Copy link
Contributor Author

megglos commented Sep 20, 2022

Thanks, looks good to me 👍

This has the drawback that we are not checking for duplicate runs across jobs but I think the original motivation was to prevent overriding test reports so that should be fine.

Yeah I thought about that too and when checking the original changes realised it's all about potential maven misconfiguration that can lead to duplicates within one build. Given that scope and that builds are split by module I would argue it's not worth going further as duplicates may only happen if a module is run multiple times.

@megglos
Copy link
Contributor Author

megglos commented Sep 20, 2022

bors merge

@zeebe-bors-camunda
Copy link
Contributor

Build succeeded:

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit a136fa8 into main Sep 20, 2022
@zeebe-bors-camunda zeebe-bors-camunda bot deleted the meg-10260-gha-duplicate-test-check branch September 20, 2022 09:37
@deepthidevaki
Copy link
Contributor

@megglos @oleschoenburg Merge of another PR failed due to duplicate tests https://github.com/camunda/zeebe/actions/runs/3089816856/jobs/4997856605#step:41:55 That PR did not add any new tests.

@megglos
Copy link
Contributor Author

megglos commented Sep 20, 2022

@deepthidevaki @oleschoenburg seems like the duplicate detection has false-positives in case of test failures

[INFO] Running io.camunda.zeebe.it.clustering.ClusteredSnapshotTest
....
Error:  Tests run: 12, Failures: 0, Errors: 4, Skipped: 0, Time elapsed: 699.334 s <<< FAILURE! - in io.camunda.zeebe.it.clustering.ClusteredSnapshotTest
Error:  ClusteredSnapshotTest.shouldSendSnapshotOnReconnect  Time elapsed: 36.561 s  <<< ERROR!

I will verify with the script and create a fix!
@deepthidevaki sorry for the inconvenvience

@deepthidevaki
Copy link
Contributor

May be due to flaky tests? Flaky tests are run more than once.

@megglos
Copy link
Contributor Author

megglos commented Sep 20, 2022

May be due to flaky tests? Flaky tests are run more than once.

indeed the log contains this twice because of flaky retries :

[INFO] Running io.camunda.zeebe.it.clustering.ClusteredSnapshotTest
....
[INFO] Running io.camunda.zeebe.it.clustering.ClusteredSnapshotTest

zeebe-bors-camunda bot added a commit that referenced this pull request Sep 20, 2022
10413: Revert "ci(gha): check for duplicate test runs" r=oleschoenburg a=megglos

## Description

This reverts commit d2c4c41 as it caused issues due to tee swallowing the maven exit code and false-positives on flaky test reruns.

## Related issues

related #10389 



Co-authored-by: Meggle (Sebastian Bathke) <sebastian.bathke@camunda.com>
@megglos
Copy link
Contributor Author

megglos commented Sep 20, 2022

@deepthidevaki thanks to Ole we realised that in this case the duplicate test detection should actually not be run at all as the build failed due to the flaky tests. The underlying issue is that this change swallowed the maven exit code and thus resulted in the detection being run still.

zeebe-bors-camunda bot added a commit that referenced this pull request Sep 20, 2022
10413: Revert "ci(gha): check for duplicate test runs" r=oleschoenburg a=megglos

## Description

This reverts commit d2c4c41 as it caused issues due to tee swallowing the maven exit code and false-positives on flaky test reruns.

## Related issues

related #10389 



Co-authored-by: Meggle (Sebastian Bathke) <sebastian.bathke@camunda.com>
@megglos
Copy link
Contributor Author

megglos commented Sep 20, 2022

fixed with #10415

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.

Detect duplicate test runs in GHA
3 participants