-
Notifications
You must be signed in to change notification settings - Fork 112
fix: Put back the limit of 50 records per notice type in the html report. #2027
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
fix: Put back the limit of 50 records per notice type in the html report. #2027
Conversation
📝 Acceptance Test Report📋 Summary❌ The rule acceptance test has failed for commit f7f4755 📊 Notices ComparisonNew Errors (0 out of 1866 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1866 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1866 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1866 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check55 out of 1921 sources (~3 %) are corrupted.
⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
📝 Acceptance Test Report📋 Summary✅ The rule acceptance has passed for commit f7f4755 📊 Notices ComparisonNew Errors (0 out of 1886 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Errors (0 out of 1886 datasets, ~0%) ✅No changes were detected due to the code change. New Warnings (0 out of 1886 datasets, ~0%) ✅No changes were detected due to the code change. Dropped Warnings (0 out of 1886 datasets, ~0%) ✅No changes were detected due to the code change. 🛡️ Corruption Check35 out of 1921 sources (~2 %) are corrupted.
⏱️ Performance Assessment📈 Validation TimeAssess the performance in terms of seconds taken for the validation process.
📜 Memory Consumption
|
davidgamez
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.
LGTM!
Summary:
Closes #2025
Reinstated the limit of 50 notices listed per notice type in the html report.
Expected behavior:
The html report should never have more than 50 notices listed per notice type.
For the sake of expediency I did not add a test case for this.
It would be way more involved since it requires parsing the resulting HTML report file.
We can add a separate issue for that.
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle testto make sure you didn't break anything