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

code coverage #306

Closed
connorjclark opened this issue Mar 5, 2019 · 3 comments
Closed

code coverage #306

connorjclark opened this issue Mar 5, 2019 · 3 comments

Comments

@connorjclark
Copy link
Contributor

seems coverage was removed? I had a hunch about some code that could be removed, and wanted to confirm if it was hit in any tests.

@fabiosantoscode
Copy link
Collaborator

Code coverage was removed for several reasons. My reason was that we would need a coverage tool for our custom test/run-tests.js machinery. If any of those tests (which make most of our tests) cover the code you want to remove, you wouldn't know even if we had coverage :(

I think adding coverage functionality to our custom testing machinery is out of scope for this project, so I have to close the issue :)

@connorjclark
Copy link
Contributor Author

Fair enough, coverage would be useful but I understand it'd be a bit of work to add to the project's test harness. For the benefit of anyone out there reading this issue and interested in doing the work, I'll ask you - would you accept a PR that added coverage?

Related to tests, I found mocha tests to be cumbersome when run via run-tests.js for a couple reasons: 1) no way to filter what tests run and 2) when an assertion fails it does not describe what differed. I only get a diff when using mocha directly. It took me awhile to figure out I should just use the mocha binary. Perhaps I should add some documentation about .node_modules/.bin/mocha test/mocha --grep ... to clear things up?

(example of above)

diff --git a/test/mocha/cli.js b/test/mocha/cli.js
index bc6c82af..d2c11475 100644
--- a/test/mocha/cli.js
+++ b/test/mocha/cli.js
@@ -35,7 +35,7 @@ describe("bin/uglifyjs", function() {
         exec(command, function (err, stdout) {
             if (err) throw err;
 
-            assert.strictEqual(stdout, "/*@preserve*/\n\n");
+            assert.strictEqual(stdout, "/*@presezrve*/\n\n");
             done();
         });
     });
yarn test

image

Stack trace is gibberish and failing assertion is not printed.

node node_modules/.bin/mocha test/mocha  --grep "Should be able to filter comments correctly with just "

image

@nylen
Copy link

nylen commented Apr 4, 2019

My reason was that we would need a coverage tool for our custom test/run-tests.js machinery.

Is there any documentation about this setup? Why it exists / why it's necessary?

would you accept a PR that added coverage?

I'd also like to know the answer to this.

Code coverage is especially useful for projects that are (a) complicated, and (b) expected to serve as a base for many other things. JavaScript minifiers meet both of these criteria in my opinion.

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

No branches or pull requests

3 participants