Skip to content

Commit

Permalink
Chore: avoid skipping test for env overrides (refs eslint#8291) (esli…
Browse files Browse the repository at this point in the history
…nt#8556)

When working on 734846b, it was discovered that the regression test for eslint#3735 was broken and always passing. The test was broken by 1e60065 over a year ago, and a regression occurred later on (possibly in 8410869), which no one noticed because the regression test was broken.

The test was supposed to assert that an environment setting in a child config will override the corresponding setting in a parent config. Unfortunately, in the time since the regression occurred, people have started using things like `parserOptions: { ecmaVersion: 7 }` along with `env: { es6: true }`, Many projects rely on the fact that when these two options are combined, `ecmaVersion: 7` will be passed to the parser even though the `env` specifies `ecmaVersion: 6`. As a result, re-fixing the original bug would cause significant ecosystem breakage.

The solution in 734846b was to skip the test and deal with the problem later. This commit modifies the test to assert the current behavior, which is the opposite of what the test was originally supposed to assert. If we decide to change the current behavior, we can always modify the test again, but I think it's important to verify that the current behavior doesn't change by mistake, because a lot of projects are relying on it.

Refs: eslint#3735, eslint#8291, eslint#8295 (comment)
  • Loading branch information
not-an-aardvark committed May 9, 2017
1 parent 456f519 commit 25db3d2
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{
"ecmaFeatures": {
"globalReturn": false
"parserOptions": {
"ecmaFeatures": {
"globalReturn": false
}
}
}
4 changes: 2 additions & 2 deletions tests/lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -773,13 +773,13 @@ describe("Config", () => {
});

describe("with env in a child configuration file", () => {
xit("should overwrite parserOptions of the parent with env of the child", () => {
it("should not overwrite parserOptions of the parent with env of the child", () => {
const config = new Config({ cwd: process.cwd() });
const targetPath = getFixturePath("overwrite-ecmaFeatures", "child", "foo.js");
const expected = {
rules: {},
env: { commonjs: true },
parserOptions: { ecmaFeatures: { globalReturn: true } }
parserOptions: { ecmaFeatures: { globalReturn: false } }
};
const actual = config.getConfig(targetPath);

Expand Down

0 comments on commit 25db3d2

Please sign in to comment.