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

Unittests fail in node 6.5.0 #2467

Closed
palortoff opened this issue Sep 2, 2016 · 4 comments
Closed

Unittests fail in node 6.5.0 #2467

palortoff opened this issue Sep 2, 2016 · 4 comments

Comments

@palortoff
Copy link

With Node 4 unittests fail:

  1. Suite .beforeAll() wraps the passed in function in a Hook adds it to _beforeAll:
  2. Suite .afterAll() wraps the passed in function in a Hook adds it to _afterAll:
  3. Suite .beforeEach() wraps the passed in function in a Hook adds it to _beforeEach:
  4. Suite .afterEach() wraps the passed in function in a Hook adds it to _afterEach:

All have a similar Error:
AssertionError: expected '"before all" hook: fn' to be '"before all" hook'

Reason:
In node 6.5.0 anomynous functions have a name when assigned to a variable:

var myFunc = function(){};
console.log(myFunc.name); // ->  'myFunc'

Prior to node 6.5.0 the function name would be empty

palortoff added a commit to palortoff/mocha that referenced this issue Sep 2, 2016
palortoff added a commit to palortoff/mocha that referenced this issue Sep 2, 2016
make function to be passed to hook truly anomynous
@ScottFreeCode
Copy link
Contributor

I have to say I'm a bit surprised by the Node change in the first place; did this change in the ECMAScript standard and I missed it? (With my focus on portable browser programming I miss a lot of cutting-edge things, so I'm not exactly going to be shocked if so, I'm just wondering.) I'm also marginally surprised it wasn't considered semver-major -- I can see where they might expect that nobody is actually relying on certain functions being literally anonymous for behavioral differences and thus more functions having names could be seen as a positive, but technically it's a deviation from the documented way functions get named up till now. (The way I see it, and I would contend the actual standard agrees with me, SemVer is primarily about API contracts [in the code sense; and hence the standard's focus on documentation] and not whether a change happens to actually break anything [which goes almost unmentioned in the standard]. Every change is going to happen to break something, but having an documented contract allows you to determine which code is incorrect by the specification and should be fixed.) Also, while I can see it being helpful for people who want to use the new arrow syntax for its own sake, I wonder if there are edge cases where the whole reason somebody assigned an anonymous function to a variable instead of using a function declaration statement is because the variable and the function aren't meant to be that tightly coupled? I guess I'd have to know more about exactly what cases Node treats as no longer anonymous.

Not that it matters much in practice; if Node decided to make all functions have the name "foobar", we'd probably need to support it.

@Munter
Copy link
Member

Munter commented Sep 4, 2016

These are the v8 release notes: http://v8project.blogspot.dk/2016/04/v8-release-51.html

Function names inferred for function expressions are now typically made available in the name property of functions, following the ES2015 formalization of these rules.

We hit the same problem in unexpected: unexpectedjs/unexpected@a69540e

@ScottFreeCode
Copy link
Contributor

Huh, so that actually is standard. Now I know!

@from-nibly
Copy link

Dang this actually is a big change for my dependency injection library that has precedence higher for the name of the function than it does for a passed in name. Now the name of the variable you assign to it becomes the func.name property...

1999 pushed a commit to 1999/mocha that referenced this issue Sep 19, 2016
…roperty-currentretry

* upstream/master:
  helpful error when necessary suite callback omitted; closes mochajs#1744
  Fix an issue and add relevant tests when describe and xdescribe fail when not supplied with a callback (issue mochajs#1744).
  rename more fixtures; closes mochajs#2383
  Report non-match to STDERR and exit if no files are matched (mochajs#2450)
  Exit process with correct error codes (mochajs#2445)
  fix test files not using .spec suffix fix test fixtures not using .fixture suffix
  always halt execution when async skip() called; closes mochajs#2465 (mochajs#2479)
  avoid test flake in "delay" test; closes mochajs#2469 (mochajs#2470)
  revert accidental change to bin paths
  disregard function names when testing under Node.js 6.5.0; closes mochajs#2467 (mochajs#2477)
  lints more files; add more files to lint check; closes mochajs#2457
  adds *.orig to .gitignore
  Exclude the --inspect flag
  fix check-leaks to catch a leak whose name begins with 'd'
1999 pushed a commit to 1999/mocha that referenced this issue Sep 22, 2016
…-files-cache

* upstream/master:
  attempt windows-friendly reproducible case for mochajs#2315
  fix: fix uncaught TypeError if error occurs on next tick, closes mochajs#2315 (mochajs#2439)
  helpful error when necessary suite callback omitted; closes mochajs#1744
  Fix an issue and add relevant tests when describe and xdescribe fail when not supplied with a callback (issue mochajs#1744).
  rename more fixtures; closes mochajs#2383
  Report non-match to STDERR and exit if no files are matched (mochajs#2450)
  Exit process with correct error codes (mochajs#2445)
  fix test files not using .spec suffix fix test fixtures not using .fixture suffix
  always halt execution when async skip() called; closes mochajs#2465 (mochajs#2479)
  avoid test flake in "delay" test; closes mochajs#2469 (mochajs#2470)
  revert accidental change to bin paths
  disregard function names when testing under Node.js 6.5.0; closes mochajs#2467 (mochajs#2477)
  lints more files; add more files to lint check; closes mochajs#2457
  adds *.orig to .gitignore
  Exclude the --inspect flag
  fix check-leaks to catch a leak whose name begins with 'd'
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

5 participants