-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
🚀 Feature: this.test.error() in "after all" hook should throw #4515
Comments
Hello, @boneskull! I decided to ping you because I think this issue is quite severe. |
I'm surprised that |
Thank you for the fast reply! I learned about that API in issue #1635. See this comment. |
edit: was not removed, just not obvious |
@boneskull You mean undocumented, right? Because it was definitely not removed. I am using it successfully right now. Edit: actually it is still documented: https://github.com/mochajs/mocha/wiki/HOW-TO:-Conditionally-fail-a-test-after-completion |
no, you're right, it still exists.. it's just that the function doesn't seem to be meaningfully tested in any way. well, we need tests for it, then we need to figure out what broke it... |
also: don't rely on the wiki for documentation if you can help it. if you don't see an API mentioned on mochajs.org, you probably don't want to use it. the API here is documented as a public API of the |
OK, so:
Still, there were no tests asserting this behavior works (I've now written one). But, we could improve the situation in two ways:
I am not sure I'd want to encourage use of this API by adding it to the official docs, but I think it would have saved you some time if it threw. If anyone is interested in helping, I think what I'd like to see is 2. above; make |
I wonder if @evaline-ju would be interested in this one |
@papb to be clear, rewriting your example as this works: const assert = require('assert');
describe('First test block', function() {
afterEach(function() {
// I know this code doesn't make sense in real life, it's just to show the issue
this.test.error(new Error());
});
it('First test', function() {
assert.strictEqual(true, true);
});
});
describe('Second test block', function() {
it('Second test', function() {
assert.strictEqual(true, true);
});
}); |
It also looks like this does not provide a reasonable stack trace. I'm going to guess that's because the error is never thrown and the stack is not created. You'd want to read the We should not change it so errors thrown in "after each" mark the test itself as failed, since that'd break the current behavior. But it's such a flimsy API atm I'm not sure building it out further makes sense. Maybe a custom interface would work. |
Hi @boneskull, thank you for taking the time to look into this so quickly! I agree with you. In fact, I only stumbled upon this obscure I opened this issue because I was worried that I ended up finding something that causes mocha to exit abruptly with exit code 0. How exactly it happens wasn't my focus; I wonder if there are other things that could also cause mocha to exit abruptly with exit code 0, since that's the scary part. In summary: what I really want is a solution to #1635, but I found this scary behavior in the process of looking for a workaround. |
Maybe it could be put into a new option, such as I also considered starting a PR to get a proper solution for #1635, but after skimming at the source code it didn't look so easy. |
By the way, I think #3345 should be reopened as well. |
yeah, dunno if that one will ever be addressed. the intent of hooks is not as a place to put assertions, though many people seem to want to. in many cases, programmatically generating tests would solve the same problems. |
Hmm, this looks interesting, can you give me a link where I can learn more? Or show an example? |
I don't think a new option is on the table. it's just not going to impact enough users to justify the maintenance cost. if you need to make the same assertions for a bunch of tests, use a loop in a describe('something', function() {
// or use a diff data structure w/ titles, like a Map
// these should be unique
const tests = [
function() {
assert.ok();
},
function() {
assert.something()
}
];
for (const t of tests) {
it('x', function() {
// may want to try/catch here
t.call(this);
assert.everyTime();
});
}
}); google for dynamic mocha tests. there might be something on the docs site; I don't recall. |
Ok! Thank you! |
I would like to add a question. |
Prerequisites
faq
labelnode node_modules/.bin/mocha --version
(Local) andmocha --version
(Global). We recommend that you not install Mocha globally.Description
Steps to Reproduce
mkdir temp && cd temp && npm init -y && npm i -D mocha
Create
test/test.js
:npx mocha test/*.js && echo 'ok'
Expected behavior: Mocha should exit with nonzero exit code and tell me that something is wrong.
Actual behavior:
Mocha completely swallowed the error and exited with exit code 0. Some tests didn't even run!
Reproduces how often: Every time
Versions
npx mocha --version
: 8.2.1node --version
: v12.14.1The text was updated successfully, but these errors were encountered: