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

Add statistics to regression.js #1932

Closed
wants to merge 2 commits into from

Conversation

johnkenny54
Copy link
Contributor

I've been using regression.js a lot locally to test the Oxygen icons and W3C files, both to isolate problems and to verify that my changes aren't having any unintended side effects.

These changes should have no impact on the regressions being run as part of CI.

Changes to regression.js:

  • Record statistics during test run (input file length, optimized file length).
  • Write a timestamped .tsv file to tmp directory at end of run (for each file, show pass/fail, orig length, optimized length, reduction in size).
  • Show total reduction in file size on console at end of tests.
  • Added some configuration constants. Eventually I would like to make these command line options. I've been using this file to track down the regression errors with the Oxygen icons and it's very helpful to be able to run the regressions on a subset of files and/or with alternate configurations.

Added tmp to .gitignore

Copy link
Member

@SethFalco SethFalco left a comment

Choose a reason for hiding this comment

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

Thanks for peeking at this. I agree the output of our regression tests could be better, but I disagree with how some of this was approached.

In particular, I'd prefer if we kept regression.js as a simple CLI tool that just reads args and writes to STDOUT, and we can work on improving that experience rather than maintaining multiple output formats.

One notable change that would be nice is to drop logs for passed tests. Given the number of tests we'll have once the updated regression tests have been merged, they won't be practical anymore.

test/regression.js Show resolved Hide resolved
test/regression.js Show resolved Hide resolved
test/regression.js Show resolved Hide resolved
test/regression.js Show resolved Hide resolved
test/regression.js Show resolved Hide resolved
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