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] Deduplicate and clean XML test reports #12332

Merged
merged 2 commits into from
Aug 8, 2022

Conversation

gigiblender
Copy link
Contributor

@gigiblender gigiblender commented Aug 7, 2022

This PR does the following:

  • When running on a shard, removes the tests that will not run on the current shard from the suite. We'll no longer get the message Test running on shard X of Y in the XML reports. This will result in cleaner test reports.
  • Adds the shard index or no-shard to the generated XML report file name. Currently, the reports generated when sharding is present are overwritten in S3 by the last shard to finish in the CI pipeline. This change is needed as part of [CI Problem] Figure out a way to assert certain tests aren't skipped in CI #11670.
  • Since the same tests might run on different configurations (CPU, GPU), it uploads the result of each configuration in a subdirectory in S3 (e.g /pytest-results/frontend_aarch64).

cc @Mousius @areusch @driazati

)
)
if item_shard_index != shard_index:
items.remove(item)
Copy link
Member

Choose a reason for hiding this comment

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

The only hesitation I have with this is that now if someone goes and looks for a test named test_abc in one of the 10 CPU integration shard logs, they'd have to search through every log until they find it and more importantly know that it's not weird that the test doesn't show up at all in most of the logs. Do you have any ideas on how we could make this a better ux?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not very sure how we could make this a better UX.
I understand your concern, but I'm wondering how often someone does a manual search for a specific test through the shard logs. If a test fails, the corresponding shard will appear red in the UI.

Alternatively, we can keep the old log structure (showing the skipped tests), and I can filter them later when processing the XML logs.

Copy link
Member

Choose a reason for hiding this comment

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

I see what you mean, I suppose it would be pretty rare. In a follow up or something we can add a message that links to the log where all the steps are catted together

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2022

Built docs for commit c229e2b can be found here.

@driazati driazati merged commit 7f800e4 into apache:main Aug 8, 2022
driazati pushed a commit that referenced this pull request Aug 8, 2022
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
This PR does the following:
- When running on a shard,  removes the tests that will not run on the current shard from the suite. We'll no longer get the message `Test running on shard X of Y` in the XML reports. This will result in cleaner test reports.
- Adds the shard index or `no-shard` to the generated XML report file name. Currently, the reports generated when sharding is present are overwritten in S3 by the last shard to finish in the CI pipeline. This change is needed as part of apache#11670.
- Since the same tests might run on different configurations (CPU, GPU), it uploads the result of each configuration in a subdirectory in S3 (e.g `/pytest-results/frontend_aarch64`).
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
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.

2 participants