-
-
Notifications
You must be signed in to change notification settings - Fork 748
Short variant of test_report.html #6034
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
Conversation
|
Ooh, I am excited about this +1 |
Unit Test Results 18 files ±0 18 suites ±0 9h 30m 20s ⏱️ - 24m 59s For more details on these failures, see this check. Results for commit ac28cfd. ± Comparison against base commit 2ff681c. ♻️ This comment has been updated with latest results. |
ian-r-rose
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks @crusaderky!
| .mypy_cache/ | ||
|
|
||
| reports/ | ||
| test_report.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will hit the short report?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed
| r = get_from_github(next_page) | ||
| artifacts = workflows + r.json()["workflow_runs"] | ||
| r = get_from_github(next_page, params=params) | ||
| artifacts += r.json()["artifacts"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I suppose we were never getting to the second page
| ndownloaded = 0 | ||
| print(f"Downloading and parsing {nartifacts} artifacts...") | ||
|
|
||
| with shelve.open("test_report") as cache: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today I learned about shelves. I guess this should do a good job of ameliorating rate limiting as well. We may still start running into rate limiting issues since this doesn't cache everything, so we should keep an eye on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this should do a good job of ameliorating rate limiting as well.
The shelf is on a local file which is lost every time you restart the test, so no.
Storing the cache somewhere more permanent would let us show the full 90 days instead of just ~20, but
- it would also require some sort of expiration logic
- regenerating the cache from scratch (for whatever reason) on the full 120 days would be challenging, as it would hit the rate limiter.
We may still start running into rate limiting issues since this doesn't cache everything
Yes, we're downloading the list of workflows and artifacts twice now. I hope that's not the tipping point...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we're downloading the list of workflows and artifacts twice now. I hope that's not the tipping point...
Ooh, this might be an issue then, we were fairly close to the rate limit as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(edit) nevermind, we're already not downloading the artifact list for workflow 51+
test_report.html is a great tool to answer the questions
However, I find it less usable to answer the questions
This PR:
Known issue:
the sticker on the README file is identical for the short and long variant of the report
Sample output:
