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

Duplicate keys throw an error with an unhelpful message #48

Closed
JaKXz opened this issue Dec 19, 2016 · 7 comments
Closed

Duplicate keys throw an error with an unhelpful message #48

JaKXz opened this issue Dec 19, 2016 · 7 comments

Comments

@JaKXz
Copy link

JaKXz commented Dec 19, 2016

I am working on stylelint webpack plugin and I think it would be appropriate for stylelint itself to inform users that they have duplicate keys in their .stylelintrc. I have been trying to get a PR going but I haven't had success running the tests on master yet.

Raising the issue here because I don't think that it's difficult to fix but I'm kinda blocked right now.

@JaKXz
Copy link
Author

JaKXz commented Jan 6, 2017

@davidtheclark what's blocking me from fixing this is the tests failing from a clean state on master when I clone and run yarn install on my system with the latest stable node (v7.4.0 at time of writing). Does this package depend on a specific version of node?

I noticed that node v0.12 support was added recently even though the LTS support dropped as of December. Are there plans to move forward again?

  • regardless, I'm quite sure that the tests should be written in ES2015+ since AVA transpiles them anyway.

@davidtheclark
Copy link
Collaborator

@JaKXz How are the tests failing? What is the error? Have you tried seeing if the tests pass on Travis with Node 7 (by changing the .travis.yml file in a PR)?

I just tried rm -rf node_modules && yarn install && npm test and had no problem. I am on Node v4.7.0, Yarn v0.18.1.

Does this package depend on a specific version of node?

As specified in the package.json, this package should work on Node 0.12 and higher.

I noticed that node v0.12 support was added recently

It was re-added, because we removed support but then some users wanted it back, and it was easy to restore.

Are there plans to move forward again?

I don't understand "move forward" here. There shouldn't be a problem with higher versions of Node. Maybe your error message will clarify things.

I'm quite sure that the tests should be written in ES2015+ since AVA transpiles them anyway.

I disagree. I'd rather have the codebase stick to one feature set than write the tests differently than the rest of the code.

@JaKXz
Copy link
Author

JaKXz commented Jan 6, 2017

@davidtheclark the failures are here: https://travis-ci.org/davidtheclark/cosmiconfig/jobs/189389524

I had spent some time a few weeks ago attempting to debug the failures locally but made no progress without context. I suppose I'll make a separate issue about the tests failing on node v7 (done: #53).

re:

I'd rather have the codebase stick to one feature set than write the tests differently than the rest of the code

using t for all assertions is the compromise that I'm suggesting, then.

@davidtheclark
Copy link
Collaborator

https://github.com/avajs/ava/blob/master/docs/common-pitfalls.md#why-are-the-enhanced-assertion-messages-not-shown

You have to name the variable t? That is stupid. I wonder if these other failures are also caused by AVA bugs. At some point I'll try switching to tape to see what happens on Node 7. Or if you want to speed things along, feel free to give that a shot.

@JaKXz
Copy link
Author

JaKXz commented Jan 6, 2017

I've been meaning to give jest a whirl, would you mind that?

@davidtheclark
Copy link
Collaborator

@JaKXz If you'd like to try, sure! It will be much more effort, though, since the syntax is more different from AVA's and the way of handling mocks will need to change. If that's the kind of challenge you seek, go for it.

@davidtheclark
Copy link
Collaborator

Closing as stale, and probably out of scope for this package.

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

2 participants