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 failing test for negative .gitignore rules #199

Closed
wants to merge 5 commits into from

Conversation

JonnyBurger
Copy link

This test demonstrates the bug that negative entries in .gitignore now get linted even if we don't include them in the list of files we want linted.

See: #195 (comment)

@JonnyBurger
Copy link
Author

Stumbled upon another issue, a test on Windows is not actually working:

test('ignore files in .gitignore', async t => {
	const cwd = path.join(__dirname, 'fixtures/gitignore');

	try {
		const result = await execa('../../../cli.js', ['--no-local'], {cwd});
	} catch (err) {
		t.is(err.stdout.indexOf('foo.js'), -1);
		t.true(err.stdout.indexOf('bar.js') !== -1);
	}
        // No error gets thrown, therefore the test passes even though it shouldn't.
});

That is why my 'failing' test ended up passing and that is why the build is broken.
Any help from more knowledgable people is appreciated.

Copy link
Contributor

@schnittstabil schnittstabil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use t.throws instead 😉

test/cli.js Outdated
await execa('../../../cli.js', ['--no-local'], {cwd});
const result = await execa('../../../cli.js', ['--no-local'], {cwd});
console.log(result, result.stdout, result.stderr);
t.fail();
} catch (err) {
t.is(err.stdout.indexOf('foo.js'), -1);
t.true(err.stdout.indexOf('bar.js') !== -1);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-       try {
-               await execa('../../../cli.js', ['--no-local'], {cwd});
-       } catch (err) {
-               t.false(err.stdout.includes('foo.js'));
-               t.true(err.stdout.includes('bar.js'));
-       }
+       const err = await t.throws(execa('../../../cli.js', ['--no-local'], {cwd}));
+       t.false(err.stdout.includes('foo.js'));
+       t.true(err.stdout.includes('bar.js'));

test/cli.js Outdated
} catch (err) {
t.is(err.stdout.indexOf('foo.js'), -1, 'Should not lint foo.js');
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const err = await t.throws(execa(`../../../cli.js`, [`${cwd}/bar.js`, '--no-local'], {cwd});
t.is(err.stdout.indexOf('foo.js'), -1, 'Should not lint foo.js');

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is cool! I didn't know that you could await t.throws and then make further assertions with the error. Thanks and I changed it!

@sindresorhus
Copy link
Member

Fixed by #204.

@sindresorhus
Copy link
Member

Thanks for the tests @JonnyBurger 🙌

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

Successfully merging this pull request may close these issues.

None yet

3 participants