Update tests to Intern 4 #356
Conversation
intern.json
Outdated
| { "browserName": "edge" }, | ||
| { "browserName": "firefox", "platform": "WINDOWS" }, | ||
| { "browserName": "chrome", "platform": "WINDOWS" } | ||
| // Issue with iphone 9.1 and Safari compatability, commented out for move to |
There was a problem hiding this comment.
Erm not especially, what is intern using to allow comments? Perhaps we should remove them.
| async function executeTest(suite: Suite, htmlTestPath: string, timeout = 10000) { | ||
| try { | ||
| return await suite.remote.get(htmlTestPath).then(pollUntil(function () { | ||
| return (<any> window).loaderTestResults || null; |
There was a problem hiding this comment.
would it be better to use dojo/shim/global?
There was a problem hiding this comment.
This function is always run on the client and it would be a big overhead to require() the shim for this simple polling function.
There was a problem hiding this comment.
is it a big overhead to require a single module from shim?
There was a problem hiding this comment.
Since this is a functional test, the text of this function is sent to the client and run synchronously. There's no way to do a require(['@dojo/shim/global'], function () { ... }) in the execution on the client for a pollUntil.
| assert.strictEqual(object.month, date.getMonth() + 1); | ||
| date.setMonth((object.month = 1) - 1); | ||
| assert.strictEqual(object.month, date.getMonth() + 1); | ||
| tests: { |
There was a problem hiding this comment.
what's the purpose of the test wrapping object?
There was a problem hiding this comment.
I've found out, that if there are lifecycle functions such as before then all tests need to be wrapped in a tests object.
tests/unit/Destroyable.ts
Outdated
| import { Destroyable } from '../../src/Destroyable'; | ||
|
|
||
| registerSuite({ | ||
| name: 'bases/Destroyable', | ||
| registerSuite('bases/Destroyable', { |
There was a problem hiding this comment.
do we need bases in the name now? Think this is a hangover from where they were moved from (compose)
There was a problem hiding this comment.
If you'd like me to update this as part of the PR, just give me the word
There was a problem hiding this comment.
as you're moving the name might be nice 👍
| assert.strictEqual(map.get([ 1 ] ), 'foo'); | ||
| assert.strictEqual(map.get([ 2 ] ), 'baz'); | ||
| }, | ||
| tests: { |
tests/unit/QueuingEvented.ts
Outdated
| registerSuite({ | ||
| name: 'Evented', | ||
|
|
||
| registerSuite('Evented', { |
| @@ -1,6 +1,3 @@ | |||
| { | |||
There was a problem hiding this comment.
can remove the file if there are no more typings required.
tsconfig.json
Outdated
| @@ -14,6 +14,12 @@ | |||
| "moduleResolution": "node", | |||
| "noImplicitAny": true, | |||
| "noImplicitThis": true, | |||
| "plugins": [ | |||
There was a problem hiding this comment.
It loads a tslint plugin so linting errors come through in the language services. We're using it for Intern 4 and makes the development experience quite nice.
There was a problem hiding this comment.
Okay. We should probably put an issue in meta if we want to add that to the Dojo 2 packages.
There was a problem hiding this comment.
rather than include it in core as part of the intern 4 conversion.
| @@ -9,6 +9,9 @@ module.exports = function (grunt) { | |||
| options: { | |||
| ignoreCompilerErrors: true, // Remove this once compile errors are resolved | |||
| } | |||
| }, | |||
| intern: { | |||
| version: 4 | |||
There was a problem hiding this comment.
We don't need this anymore with your new grunt-dojo2 changes, right?
There was a problem hiding this comment.
grunt-dojo2 still defaults to Intern 3 for backwards compatibility, so we need this
Type: enhancement
The following has been addressed in the PR:
Description:
This PR updates the unit and functional tests from Intern 3 to Intern 4
Resolves #347