search

AtomLinter / linter-eslint-node

ESLint plugin for Atom/Pulsar Linter (v8 and above)
https://web.pulsar-edit.dev/packages/linter-eslint-node
MIT License
4 stars 3 forks source link

test: set cwd based on ESLint directory #14

Closed brettz9 closed 2 years ago

brettz9 commented 2 years ago

test: set cwd based on ESLint directory; fixes #13

savetheclocktower commented 2 years ago

I’m on a quick vacation, but I’ll take a look at this over the weekend.

savetheclocktower commented 2 years ago

Thanks for this.

I had one hangup here: the process.chdir call into the new CWD is happening inside getESLint in a code path that won't get hit if we're reusing a cached instance. So it wouldn't be hard to encounter a scenario where the directory doesn't get changed.

Whether that matters depends on when ESLint actually consults .eslintrc.js. I would've bet $5 that it happened upon instantiation, and that we'd only need to change CWD long enough to create our ESLint instances. But I did some testing, and I'm wrong: if I put a logging message in that .eslintrc.js fixture, I can see that it isn't actually evaluated until a specific linting task is performed.

So we don't need to chdir at all during instantiation; we just need to chdir before and after a linting/fixing job. I decided to move it above where we handle debug jobs just in case we ever decide to do anything CWD-dependent as part of a debug.

Dynamic behavior in .eslintrc.js gives me the willies, but I've got no legitimate argument against it; it works in the eslint CLI, after all. I'd hoped the cwd option in the ESLint constructor would behave identically in all respects and I wouldn't have to deal with process.chdir at all, but them's the breaks.

savetheclocktower commented 2 years ago

The CI failure on Windows cannot possibly be related to this change, so I'm going to land this PR later today. If it ever fails again I'll do whatever I need to to make it un-flaky.

github-actions[bot] commented 2 years ago

:tada: This PR is included in version 1.0.4 :tada:

The release is available on:

Your semantic-release bot :package::rocket: