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

False positive for no-hooks-from-ancestor-modules #230

Closed
Krinkle opened this issue Apr 27, 2022 · 1 comment · Fixed by #231
Closed

False positive for no-hooks-from-ancestor-modules #230

Krinkle opened this issue Apr 27, 2022 · 1 comment · Fixed by #231

Comments

@Krinkle
Copy link
Contributor

Krinkle commented Apr 27, 2022

QUnit.module( 'jquery.tablesorter.parsers', QUnit.newMwEnvironment(), function ( hooks ) {
	var originalMonths;
	hooks.beforeEach( function () {
	// ^ eslint: Do not call hooks.{{hookName}} from an ancestor module.
		originalMonths = mw.language.months;
		mw.language.months = {};
	} );
	hooks.afterEach( function () {
	// ^ eslint: Do not call hooks.{{hookName}} from an ancestor module.
		mw.language.months = originalMonths;
	} );
	
	QUnit.test( 'example', function ( assert ) {
		assert.true( true );
	} );
} );

I thought maybe it gets confused by the newMwEnvironment call that returns an options object, but it happens even if I replace it with a plain QUnit.module('example', {}, function ( hooks ) {.

@platinumazure
Copy link
Owner

Thanks @Krinkle for the issue!

Having reviewed the QUnit.module documentation, I can confidently say this is a bug. I think during initial implementation, we simply forgot that it is valid to have both a hooks object as second argument and a function callback as third argument. Instead, we check the second argument regardless.

PR would be welcome. It should be pretty easy to just add the missing test cases and then fix the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants