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

Add tests for parent directory files input #3273

Closed
regseb opened this issue Apr 19, 2018 · 7 comments · Fixed by #6398
Closed

Add tests for parent directory files input #3273

regseb opened this issue Apr 19, 2018 · 7 comments · Fixed by #6398
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: tests an improvement to testing

Comments

@regseb
Copy link
Member

regseb commented Apr 19, 2018

Hello,

Since stylelint 9.2.0, path with ".." (parent directory) are ignored ; like this example :

const config = {
    "rules": { "color-hex-case": "upper" }
};
stylelint.lint({ "files": "../style.css", config }).then(...);

To reproduce the problem, download attach file testcase.tar.gz and execute command :

tar zxvf testcase.tar.gz
cd testcase/folder
npm install
./testcase.js

Output on stylelint 9.2.0 :

../style.css { errored: false, output: '[]', results: [] }
style.css { errored: true,
output: '[{"source":"/home/regseb/dev/testcase/folder/style.css","deprecations":[],"invalidOptionWarnings":[],"parseErrors":[],"errored":true,"warnings":[{"line":2,"column":12,"rule":"color-hex-case","severity":"error","text":"Expected \"#ffffff\" to be \"#FFFFFF\" (color-hex-case)"}]}]',
results:
[ { source: '/home/regseb/dev/testcase/folder/style.css',
deprecations: [],
invalidOptionWarnings: [],
parseErrors: [],
errored: true,
warnings: [Array],
ignored: undefined,
_postcssResult: [Result] } ] }

Output on stylelint 9.1.3 :

style.css { errored: true,
output: '[{"source":"/home/regseb/dev/testcase/folder/style.css","deprecations":[],"invalidOptionWarnings":[],"parseErrors":[],"errored":true,"warnings":[{"line":2,"column":12,"rule":"color-hex-case","severity":"error","text":"Expected \"#ffffff\" to be \"#FFFFFF\" (color-hex-case)"}]}]',
results:
[ { source: '/home/regseb/dev/testcase/folder/style.css',
deprecations: [],
invalidOptionWarnings: [],
parseErrors: [],
errored: true,
warnings: [Array],
ignored: undefined,
_postcssResult: [Result] } ] }
../style.css { errored: true,
output: '[{"source":"/home/regseb/dev/testcase/style.css","deprecations":[],"invalidOptionWarnings":[],"parseErrors":[],"errored":true,"warnings":[{"line":2,"column":12,"rule":"color-hex-case","severity":"error","text":"Expected \"#ffffff\" to be \"#FFFFFF\" (color-hex-case)"}]}]',
results:
[ { source: '/home/regseb/dev/testcase/style.css',
deprecations: [],
invalidOptionWarnings: [],
parseErrors: [],
errored: true,
warnings: [Array],
ignored: undefined,
_postcssResult: [Result] } ] }

@hudochenkov hudochenkov added type: bug a problem with a feature or rule status: needs investigation triage needs further investigation labels Apr 19, 2018
@hudochenkov
Copy link
Member

Thanks for the report and for providing a test case.

Indeed, there is a regression. I don't know what caused it. It requires further investigation.

@ntwb
Copy link
Member

ntwb commented Apr 22, 2018

After some sleuthing with git bisect the cause is commit 7208a87, your testcase works perfectly up unitl that commit @regseb

That commit turns out to be an update to globby to version 8.0.0 in #3167, now I just need to deermine if it was caused by stylelint or a globby regression.

p.s. Thanks a bunch for the testcase @regseb, extremely helpful 😄

@ntwb
Copy link
Member

ntwb commented Apr 22, 2018

That was quick, indeed it's a globby issue sindresorhus/globby#80, they are waiting for an upstream fix before they can release, I've subscribed to the upstream issues and as soon as its resolved I'll test again, due to semantic versioning fingers crossed an npm update will resolve the issue once resolved upstream.

We will also release a patch version of stylelint anyways just to be sure, this might affect more stylelint users though are unaware as the issue is not causing "more" errors and warnings, it's causing less so they might not be being detected at all.

@ntwb ntwb added pr: blocked is blocked by another issue or pr and removed status: needs investigation triage needs further investigation labels Apr 22, 2018
@mrmlnc
Copy link

mrmlnc commented Apr 22, 2018

Must be fixed by fast-glob@2.2.1. Please try to reinstall dependencies with npm i --force or npm i globby --force.

@ntwb
Copy link
Member

ntwb commented Apr 24, 2018

@mrmlnc Thanks for updating fast-glob` and thanks for the update here :)

@regseb Could you test this out please and let me know how it goes, we can create a new stylelint release if need be but I think npm update should cover this, if not try the suggestion above from @mrmlnc to force the update

@regseb
Copy link
Member Author

regseb commented Apr 24, 2018

I test with npm update : stylelint 9.2.0, globby 8.0.1 and fast-glob 2.2.1. The test case and the real case work.

Thanks.

@regseb regseb closed this as completed Apr 24, 2018
@ntwb ntwb added PR: needs tests status: ready to implement is ready to be worked on by someone good first issue is good for newcomers and removed pr: blocked is blocked by another issue or pr type: bug a problem with a feature or rule labels Apr 24, 2018
@ntwb
Copy link
Member

ntwb commented Apr 24, 2018

Thanks @regseb, I'm going to reopen this issue so that we can add some tests to detect this issue in the future, that way if it happens again we'll be notified before, rather than after 😄

@ntwb ntwb reopened this Apr 24, 2018
@jeddy3 jeddy3 added type: tests an improvement to testing and removed PR: needs tests labels Apr 25, 2018
@jeddy3 jeddy3 changed the title Path with ".." (parent directory) are ignored since 9.2.0 Add tests for parent directory files input May 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue is good for newcomers status: ready to implement is ready to be worked on by someone type: tests an improvement to testing
Development

Successfully merging a pull request may close this issue.

5 participants