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.

Sign up for GitHub

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

Jump to bottom

Core: Create config.testIds to expose all available test ids #1424

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Core: Create config.testIds to expose all available test ids #1424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gruntfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ module.exports = function( grunt ) {
"test/reporter-html/xhtml-single-testid.xhtml",
"test/reporter-urlparams.html",
"test/moduleId.html",
"test/testIds.html",
"test/onerror/inside-test.html",
"test/onerror/outside-test.html",
"test/only.html",
Expand Down
4 changes: 4 additions & 0 deletions docs/config/QUnit.config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ Enabling this option will cause tests to fail, if they don't call `expect()` at

This property allows QUnit to run specific tests identified by the hashed version of their module name and test name. You can specify one or multiple tests to run.

### `QUnit.config.testIds` (array) | default: `array`

This property allows QUnit to return all hashed testIds of all loaded tests.

### `QUnit.config.testTimeout` (number) | default: `undefined`

Specify a global timeout in milliseconds after which all tests will fail with an appropriate message. Useful when async tests aren't finishing, to prevent the testrunner getting stuck. Set to something high, e.g. 30000 (30 seconds) to avoid slow tests to time out by accident.
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 22 additions & 2 deletions src/core/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { window, localSessionStorage } from "../globals";
import { extend } from "./utilities";
import { extend, generateHash, uniqueTestName } from "./utilities";

/**
* Config object: Maintain internal state
Expand Down Expand Up @@ -58,7 +58,27 @@ const config = {
callbacks: {},

// The storage module to use for reordering tests
storage: localSessionStorage
storage: localSessionStorage,

// Get the testIds for all tests
// Simulates how test.js generates testIds by
// iteratively pushing testNames to get uniqueTestName
get testIds() {
Copy link
Member

@Krinkle Krinkle Jan 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@trentmwillis I have a vague sense that using a getter here may bite us long-term in terms of confusion and flexibility. What's your take on this? Would making this a regular function be easier to understand/document/maintain long-term? Drawback is that it introduces a non-serialisable property to QUnit.config. On the other hand, if config does get serialised, the value it would serialise to is likely wrong by the time it is consumed.

Maybe that's a reason to mount it somewhere else entirely, e.g. on currentModule?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just trying to get some traction here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@trentmwillis Any thoughts? I'd love to see this submitted to help tests run faster in parallel.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I've been busy working through other OSS tickets. From a brief glance, I think I'm inclined to agree that it doesn't belong on QUnit.config. I'll review more thoroughly this Friday.

Copy link
Member

Choose a reason for hiding this comment

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

So after reviewing more thoroughly, I think this should just be a normal property that is an array.

The testId for each test is already generated eagerly (i.e., when you call QUnit.test). The information is already available within the (undocumented) QUnit.config.modules property but would require some munging. So instead, we could set this up such that as each testId gets initialized, we push it into a new QUnit.config.testIds property.

var i, j, ml, tl, testIds = [], testNames, testName;

// Loop through all modules and tests and generate testIds
for ( i = 0, ml = config.modules.length; i < ml; i++ ) {
testNames = [];
for ( j = 0, tl = config.modules[ i ].tests.length; j < tl; j++ ) {
testName = uniqueTestName( testNames,
config.modules[ i ].tests[ j ].name );
kltan marked this conversation as resolved.
Show resolved Hide resolved
testIds.push( generateHash( config.modules[ i ].name, testName ) );
testNames.push( testName );
}
}
return testIds;
}

};

// take a predefined QUnit.config and extend the defaults
Expand Down
11 changes: 11 additions & 0 deletions src/core/utilities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,14 @@ export function generateHash( module, testName ) {

return hex.slice( -8 );
}

// Append whitespace to duplicated seen test name
export function uniqueTestName( testNames, testName ) {
var i, l;
for ( i = 0, l = testNames; i < l.length; i++ ) {
if ( testNames[ i ] === testName ) {
testName += " ";
}
}
return testName;
}
19 changes: 11 additions & 8 deletions src/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
hasOwn,
inArray,
now,
objectType
objectType,
uniqueTestName
} from "./core/utilities";
import { runLoggingCallbacks } from "./core/logging";
import { extractStacktrace, sourceFromStacktrace } from "./core/stacktrace";
Expand All @@ -26,7 +27,7 @@ import TestReport from "./reports/test";
let focused = false;

export default function Test( settings ) {
var i, l;
var testNames;

++Test.count;

Expand Down Expand Up @@ -62,12 +63,14 @@ export default function Test( settings ) {
valid: this.valid()
} );

// Register unique strings
for ( i = 0, l = this.module.tests; i < l.length; i++ ) {
if ( this.module.tests[ i ].name === this.testName ) {
this.testName += " ";
}
}
// Register unique string for each test inside a module
//
// Note: this is the authoratative way of generating testId
// testIds are generated one at a time when individual test is being run
// config.testIds also simulates the process here by pushing the test name
// one at a time to match the uniqueTestName generation here
testNames = this.module.tests.map( test => test.name );
this.testName = uniqueTestName( testNames, this.testName );

this.testId = generateHash( this.module.name, this.testName );

Expand Down
13 changes: 13 additions & 0 deletions test/testIds.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>QUnit ModuleId Test Suite</title>
<link rel="stylesheet" href="../dist/qunit.css">
<script src="../dist/qunit.js"></script>
<script src="testIds.js"></script>
</head>
<body>
<div id="qunit"></div>
</body>
</html>
10 changes: 10 additions & 0 deletions test/testIds.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
QUnit.module( "QUnit.config.testIds" );

QUnit.test( "testIds should return all testIds", function( assert ) {
assert.deepEqual( QUnit.config.testIds, [ "01e80961", "3b1922df" ], "this is the test with id 01e80961" );
} );

// This test is to prove identical test name is being hashed differently
QUnit.test( "testIds should return all testIds", function( assert ) {
assert.deepEqual( QUnit.config.testIds, [ "01e80961", "3b1922df" ], "this is the test with id 3b1922df" );
} );