Tests for trailing commas in function arg lists#733
Conversation
|
Looks like there are some good tests missing
it also might be a bit confusing to mix the Arguments trailing commas and FormalParameters trailing commas tests in the same file, but I don't have that much of an opinion on that |
|
we also need to structure these tests under the existing folders for the functions forms including those listed by @caitp |
Good catch! Will add
Can you point me to which directories this would be? I wasn't sure if there was a better place for these (or if I named the test file in an acceptable way?) |
|
sure!
It looks like a lot of tests, but they might be easier if you use the test generation tool available within the repo. |
|
Would you mind adding some tests making sure the arguments object is unaffected? |
| // Copyright (C) 2016 Jeff Morrison. All rights reserved. | ||
| // This code is governed by the BSD license found in the LICENSE file. | ||
| /*--- | ||
| description: Check that trailing commas are permitted in function argument lists |
There was a problem hiding this comment.
Similar for other descriptions, which should be a bit more descriptive about the individual test's high level purpose
There was a problem hiding this comment.
Ok. I've updated the generator descriptions to include "generator".
What level of clarity are you looking for for other descriptions though? The current descriptions seem pretty clear to me, but happy to add additional context where necessary.
There was a problem hiding this comment.
Sure, it doesn't have to be as detailed as Mike's tests for example, but something like #735 is a good example.
In this particular case documenting the type of function and/or why the test is interesting (e.g. rest) would be helpful.
| assert.sameValue( | ||
| ((a,) => {}).length, | ||
| 1, | ||
| "Arrow function with 1 arg + trailing comma reports incorrect .length1" |
There was a problem hiding this comment.
"length1" (typo -- willfix)
jeffmo
force-pushed
the
master
branch
2 times, most recently
from
5eee695 to
e4ab353
Compare
Adds tests for the proposal as described here: http://jeffmo.github.io/es-trailing-function-commas/
|
Ok, I've gone through the descriptions and tried to be more specific. Let me know what you think |
|
Looks good! Thanks for the update. Would @caitp or @leobalter mind doing a second signoff? |
|
Rubberstamp --- I think there are good tests here, which are not tested for all the variants. I am a fan of exhaustive tests, but it is a lot of effort I guess =) |
4 participants
Adds tests for the proposal as described here:
http://jeffmo.github.io/es-trailing-function-commas/
cc @bterlson @ljharb