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

this.skip() completely broken if html reporter used. #1760

Closed
Kirill89 opened this issue Jun 20, 2015 · 16 comments · Fixed by #2081
Closed

this.skip() completely broken if html reporter used. #1760

Kirill89 opened this issue Jun 20, 2015 · 16 comments · Fixed by #2081
Labels
status: waiting for author waiting on response from OP - more information needed

Comments

@Kirill89
Copy link

Demo: https://github.com/Kirill89/mocha-test

Error: TypeError: test.err is undefined (http://localhost:1111/node_modules/mocha/mocha.js:2786)

  • Firefox 38.0.5
  • Chrome 43.0.2357.124 (64-bit)
@boneskull
Copy link
Member

what are you trying to accomplish by skipping the hook?

@boneskull boneskull added the status: waiting for author waiting on response from OP - more information needed label Jul 6, 2015
@Kirill89
Copy link
Author

Kirill89 commented Jul 6, 2015

@boneskull I'm trying skip whole "describe" section.

@Kirill89
Copy link
Author

@boneskull mocha already has same test, and it is works for reporter "spec". Can you explain me why doesn't work for "html" reporter?

@danielstjules
Copy link
Contributor

Looks like you're right. Avoiding async hooks/specs, I was able to replicate in the browser:

describe('foo', function () {
  before(function() {
    this.skip();
  });

  it('bar', function() {
    // test
  });
});

From the console:

  foo
    - bar


  0 passing (9ms)
  1 pending

But in the browser:

screen shot 2015-07-15 at 2 39 57 am

So seems like it's not being correctly labeled as pending.

@danielstjules
Copy link
Contributor

@Kirill89 Are you able to get this.skip() to work from a spec, in the browser?

describe('foo', function () {
  it('bar', function() {
    this.skip();
  });
});

@Kirill89
Copy link
Author

@danielstjules I am surprised, but it is also doesn't work. But with another error.

@danielstjules
Copy link
Contributor

Thanks for confirming! Both errors are being propagated by the HTML reporter due to it trying to act on a nonexistent err property for the test, since it wasn't correctly marked as pending. Will look into where that regression may have been introduced. Appreciate your help!

@SerkanSipahi
Copy link

really nice feature this.skip( what I was looking for ) in hooks(before, after, etc). My question: why is that not documented? it there more nice magic features what is not documented? :)

@puzrin
Copy link

puzrin commented Aug 5, 2015

@danielstjules any chances to get it fixed soon?

@Kirill89 Kirill89 changed the title this.skip() in before statement seems broken this.skip() completely broken if html reporter used. Dec 27, 2015
@puzrin
Copy link

puzrin commented Dec 27, 2015

Could you change a label? Needs-Feedback -> Bug?

@puzrin
Copy link

puzrin commented Dec 27, 2015

Problem is with this check

} else if (test.pending) {

If i change this line to

   } else if (test.pending || !test.err) {

...then code seems to work. But i'm not sure such hack is correct. Could anyone check why .pending flag is lost in test end event after this.skip()?

@mislav
Copy link
Contributor

mislav commented Jan 10, 2016

This will be fixed by #1945 (comment)

@puzrin
Copy link

puzrin commented Jan 10, 2016

Thanks for info!

@puzrin
Copy link

puzrin commented Jan 27, 2016

Still not fixed with 2.4.3. Test from the first message does not shows error on screen, but throw errors to console and mark test as failed in summary.

Something become better, but not all.

@danielstjules
Copy link
Contributor

You're right - this.skip() is mostly broken in the browser. I haven't gotten around to it though.

@puzrin
Copy link

puzrin commented Jan 27, 2016

It seems to be fixed for it, but not for before (most demanded)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting for author waiting on response from OP - more information needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants