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

[DevTSAN][E2E] Run test with deflake script to make positive tests more robust #17483

Merged
merged 4 commits into from
Mar 19, 2025

Conversation

zhaomaosu
Copy link
Contributor

Also disable two unexpected flaky negative tests for further investigation

…re robust

Also disable two unexpected flaky negative tests for further investigation
@zhaomaosu zhaomaosu requested a review from a team March 17, 2025 07:33
@zhaomaosu
Copy link
Contributor Author

@intel/dpcpp-devops-reviewers, @intel/llvm-reviewers-runtime, could you please help review this PR? Thanks.

@aelovikov-intel
Copy link
Contributor

https://llvm.org/docs/CommandGuide/lit.html#test-status-results:

FLAKYPASS

The test succeeded after being re-run more than once. This only applies to tests containing an ALLOW_RETRIES: annotation.

Can you please check if that would work for you? Unfortunately, this feature doesn't have any other documentation (at least I was unable to find out), so you'd need to do some work here.

@zhaomaosu
Copy link
Contributor Author

Can you please check if that would work for you? Unfortunately, this feature doesn't have any other documentation (at least I was unable to find out), so you'd need to do some work here.

Hi @aelovikov-intel, thanks for your information. I tried 'FLAKYPASS' locally, it works here. But there is problem that 'FLAKYPASS' will retry whole testcase (including build and run) when meet fail, it will waste a lot of unnecessary time (we only need to retry run). So I think current solution to use deflake script is more suitable for sycl/test-e2e.

@aelovikov-intel
Copy link
Contributor

In pre-commit CI we run build/run of e2e tests separately... Also, can you look into implementing something like these retries for "run"-only RUN lines in e2e's format.py instead?

@zhaomaosu
Copy link
Contributor Author

In pre-commit CI we run build/run of e2e tests separately...

@aelovikov-intel, updated the PR to use "ALLOW_RETRIES".

Also, can you look into implementing something like these retries for "run"-only RUN lines in e2e's format.py instead?

The retry's work is controlled by lit.TestRunner.py https://github.com/intel/llvm/blob/sycl/llvm/utils/lit/lit/TestRunner.py#L2268. It's test level instead of command level. So if we want to implement it in format.py, the most obvious way is to substitute '%{run}' with something like deflake script do when ALLOW_RETRIES is specified. Not sure if this is what you are expecting.

@aelovikov-intel aelovikov-intel merged commit d865c0c into intel:sycl Mar 19, 2025
25 of 26 checks passed
@aelovikov-intel
Copy link
Contributor

I was too late to spot that title needed an update... :(

@zhaomaosu
Copy link
Contributor Author

I was too late to spot that title needed an update... :(

Sorry, I also forgot to update it...

@zhaomaosu zhaomaosu deleted the tsan-fix-flaky branch March 19, 2025 14:08
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.

3 participants