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

QUnit.module.only behaves strange #1272

Closed
sechel opened this issue Apr 2, 2018 · 4 comments · Fixed by #1527
Closed

QUnit.module.only behaves strange #1272

sechel opened this issue Apr 2, 2018 · 4 comments · Fixed by #1527
Assignees
Labels
Component: Core For module, test, hooks, and reporters. Type: Bug Something isn't working right.
Milestone

Comments

@sechel
Copy link

sechel commented Apr 2, 2018

QUnit.module.only behaves strange when using the module syntax that assigns tests to modules using the order of invocation, e.g.:

QUnit.module.only('module 1');

QUnit.test('test in module 1', assert => {
  assert.ok(true, 'a successful assertion in module 1');
});

QUnit.module('module 2');

QUnit.test('test in module 2', assert => {
  assert.ok(true, 'a successful assertion in module 2');
});

Without only test in module 1 belongs to module 1 and test in module 2 belongs to module 2. Using only in module 1 the second test also seems to belong to module 1 and is run (applying the wrong hooks if there would be any).

You can check this in this plunk: https://embed.plnkr.co/mh4VgJ0O6Dhp4H5APd3S/

@trentmwillis
Copy link
Member

Thanks for the report + reproduction! Definitely a bug and likely just a combination we didn't test (I think all the tests use the module callback form).

@trentmwillis trentmwillis added the Type: Bug Something isn't working right. label Apr 11, 2018
@Arkni
Copy link
Contributor

Arkni commented Jun 3, 2018

Hi,

I did some analysis and what I came with is the following: we can flag the current module (which is the one we attach only to) as focused by adding a new property to it. This property will be set to true as soon as the first ignored module is executed, and will then be used in conjuction with focused (inside test.js) to ignore tests of other modules.

In summary, here is the diff:

`src/core.js`
diff --git a/src/core.js b/src/core.js
index b2e9472..95230e5 100644
--- a/src/core.js
+++ b/src/core.js
@@ -115,6 +115,10 @@ function processModule( name, options, executeNow, modifiers = {} ) {
 // TODO: extract this to a new file alongside its related functions
 function module( name, options, executeNow ) {
 	if ( focused ) {
+		if ( !config.currentModule.focused ) {
+			config.currentModule.focused = true;
+		}
+
 		return;
 	}
 
@@ -130,6 +134,10 @@ function module( name, options, executeNow ) {
 
 module.only = function() {
 	if ( focused ) {
+		if ( !config.currentModule.focused ) {
+			config.currentModule.focused = true;
+		}
+
 		return;
 	}
 
@@ -143,6 +151,10 @@ module.only = function() {
 
 module.skip = function( name, options, executeNow ) {
 	if ( focused ) {
+		if ( !config.currentModule.focused ) {
+			config.currentModule.focused = true;
+		}
+
 		return;
 	}
 
@@ -158,6 +170,10 @@ module.skip = function( name, options, executeNow ) {
 
 module.todo = function( name, options, executeNow ) {
 	if ( focused ) {
+		if ( !config.currentModule.focused ) {
+			config.currentModule.focused = true;
+		}
+
 		return;
 	}
`src/test.js`
diff --git a/src/test.js b/src/test.js
index 097fd39..e5ab69a 100644
--- a/src/test.js
+++ b/src/test.js
@@ -658,7 +658,7 @@ function checkPollution() {
 
 // Will be exposed as QUnit.test
 export function test( testName, callback ) {
-	if ( focused ) {
+	if ( focused || config.currentModule.focused ) {
 		return;
 	}
 
@@ -671,7 +671,7 @@ export function test( testName, callback ) {
 }
 
 export function todo( testName, callback ) {
-	if ( focused ) {
+	if ( focused || config.currentModule.focused ) {
 		return;
 	}
 
@@ -686,7 +686,7 @@ export function todo( testName, callback ) {
 
 // Will be exposed as QUnit.skip
 export function skip( testName ) {
-	if ( focused ) {
+	if ( focused || config.currentModule.focused ) {
 		return;
 	}
 
@@ -700,7 +700,7 @@ export function skip( testName ) {
 
 // Will be exposed as QUnit.only
 export function only( testName, callback ) {
-	if ( focused ) {
+	if ( focused || config.currentModule.focused ) {
 		return;
 	}

If this solution looks good, I'll be happy to open a PR.

@smcclure15
Copy link
Member

Resolved with #1436?

@Arkni
Copy link
Contributor

Arkni commented Jul 12, 2020

@smcclure15

Just tested with latest version and it doesn't seem to be fixed. See https://plnkr.co/edit/lLBZUFu7opjXpTVq?preview

@Krinkle Krinkle added the Component: Core For module, test, hooks, and reporters. label Aug 22, 2020
@Krinkle Krinkle self-assigned this Aug 22, 2020
@Krinkle Krinkle removed their assignment Nov 13, 2020
smcclure15 added a commit to smcclure15/qunit that referenced this issue Dec 8, 2020
Krinkle pushed a commit that referenced this issue Jan 10, 2021
This bug affected use of QUnit.module.only when used in a "flat" way,
e.g. without scoping. For the following, it resulted in running test 1A and
2A, instead of only 1A:

```
QUnit.module.only( "Module one" );
QUnit.test( "1A", assert => { assert.ok( true ); } );

QUnit.module( "Module two" );
QUnit.test( "2A", assert => { assert.ok( false ); } );
```

Also move the "only" tests to CLI suite to verify TAP output
(more stable than `QUnit.done` post-run hack in HTML).

Fixes #1272.
@Krinkle Krinkle added this to the 2.x release milestone Jan 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core For module, test, hooks, and reporters. Type: Bug Something isn't working right.
Development

Successfully merging a pull request may close this issue.

5 participants