Create test command #1

maier49 · 2017-01-11T09:20:00Z

Delegates to cli-build to build tests and then runs them using intern-runner.

dylans

Overall this looks really helpful, mostly just added various meta comments for things I noticed. I do wonder if we want to add support for some of the Intern 3.4 features (performance, a11y, and visual regression testing) as options, though that's probably best done via a separate issue?

dylans · 2017-01-11T11:40:55Z

README.md

+[![npm version](https://badge.fury.io/js/dojo-cli-test-intern.svg)](http://badge.fury.io/js/dojo-cli-test-intern)
+-->
+
+The official dojo 2 test command.


Dojo 2 (capitalization)

dylans · 2017-01-11T11:41:19Z

README.md

+
+Test cases MUST be written using [Intern](https://theintern.github.io) using the Object test interface and Assert assertion interface.
+
+90% branch coverage MUST be provided for all code submitted to this repository, as reported by istanbul’s combined coverage results for all supported platforms.


Istanbul (capitalization)

dylans · 2017-01-11T11:42:19Z

README.md

+
+* [Third-party lib one](https//github.com/foo/bar) ([New BSD](http://opensource.org/licenses/BSD-3-Clause))
+
+© 2004–2016 Dojo Foundation & contributors. [New BSD](http://opensource.org/licenses/BSD-3-Clause) license.


Should now be JS Foundation. Should just link to our own LICENSE file probably.

dylans · 2017-01-11T11:42:43Z

LICENSE

+The "New" BSD License
+*********************
+
+Copyright (c) 2015 - 2016, The Dojo Foundation


2017 and JS Foundation.

dylans · 2017-01-11T11:43:18Z

package.json

+{
+  "name": "dojo-cli-test-intern",
+  "version": "2.0.0-pre",
+  "description": "test a dojo 2 application",


Test a Dojo 2 application

dylans · 2017-01-11T11:46:36Z

src/intern/intern-saucelabs.js

+        { browserName: 'firefox', version: '43', platform: 'Windows 10' },
+        { browserName: 'chrome', platform: 'Windows 10' },
+        // { browserName: 'safari', version: '9', platform: 'OS X 10.11' },
+        { browserName: 'android', platform: 'Linux', version: '4.4', deviceName: 'Google Nexus 7 HD Emulator' }// ,


Should we also be testing Chrome for Android separately?

I commented this out actually because I don't believe the issue with android on saucelabs have been resolved yet.

dylans · 2017-01-11T11:47:58Z

src/intern/intern.js

+        { browserName: 'internet explorer', version: [ '10', '11' ], platform: 'WINDOWS' },
+        { browserName: 'firefox', platform: 'WINDOWS' },
+        { browserName: 'chrome', platform: 'WINDOWS' }/*,
+		 { browserName: 'Safari', version: '9', platform: 'OS X' }*/


Maybe a few more examples should be commented out, e.g other browsers for Mac or Linux?

dylans · 2017-01-11T11:51:26Z

src/main.ts

+	register(helper: Helper) {
+		helper.yargs.option('c', {
+			alias: 'config',
+			describe: 'Specifies what configuration to test with: browserstack(default), \'saucelabs\', or \'local\'ly.',


could also be testingbot or crossbrowsertesting (though perhaps not since we don't include configs for those environments, but perhaps we should)?

I added configuration for testingbot. It might be documented somewhere but I couldn't see how to set testingbot credentials via environment variables so I also added command line args to specify credentials, and added the same support for browserstack and saucelabs as well.

crossbrowsertesting is not supported by the version of dig dug and intern that the create app cli sets as the application's dependency so I'm thinking we should hold off on adding support for that here until it'll work.

dylans · 2017-01-11T11:55:23Z

tsconfig.json

@@ -0,0 +1,20 @@
+{
+	"version": "2.0.3",


Any reason to not be 2.1.4?

Nope, updated.

dylans · 2017-01-11T11:55:42Z

tsconfig.json

+		"declaration": false,
+		"module": "umd",
+		"noImplicitAny": true,
+		"noImplicitThis": true,


I think there are a couple of other noImplicit* statements we've started to use?

I'm not sure. I think the last change was to add strictNullChecks: https://github.com/dojo/widgets/blob/master/tsconfig.json#L4-L15. Although if there are more that I should add I can of course put those in.

The others I'm thinking of are noUnusedParameters and noUnusedLocals , but I'm not sure if we've standardized on using those across the Dojo 2 packages? cc @agubler

We are not using those because those are captured by linting.

matt-gadd · 2017-01-11T18:29:47Z

src/intern/intern-local.js

+define([
+	'./intern'
+], function (intern) {
+    intern.tunnel = 'NullTunnel';


do we want to potentially use the SeleniumTunnel for local now?

matt-gadd · 2017-01-11T18:30:15Z

src/intern/intern-local.js

@@ -0,0 +1,15 @@
+define([


this file seems to mix tabs and spaces

…cription

agubler · 2017-01-16T17:57:36Z

README.md

+
+The official Dojo 2 test command.
+
+## Features


think we could fill out the readme a bit?

agubler · 2017-01-16T17:57:59Z

package.json

@@ -0,0 +1,44 @@
+{
+  "name": "dojo-cli-test-intern",


@dojo/cli-test-intern

agubler · 2017-01-16T17:58:29Z

src/intern/intern-saucelabs.js

+		// { browserName: 'microsoftedge', platform: 'Windows 10' },
+		{ browserName: 'firefox', version: '49', platform: 'Windows 10' },
+		{ browserName: 'chrome', platform: 'Windows 10' },
+		// { browserName: 'safari', version: '9', platform: 'OS X 10.11' },


worth just removing the commented out platforms?

@dylans requested I put in more commented out platforms as examples. I can see the usefulness of having some examples commented out so you can add some more browsers without having to go check saucelabs for the format.

but these configs are hidden from the consumer, so I'm not sure what benefit this will have.

That's a really good point. I'll remove them

agubler · 2017-01-16T17:59:30Z

src/main.ts

+	run(helper: Helper, args: TestArgs) {
+		return new Promise((resolve, reject) => {
+			if (!helper.command.exists('build')) {
+				reject(Error('Required command: \'build\', does not exist'));


think we could say, have you npm install @dojo/cli-build? as part of the error?

agubler · 2017-01-16T18:02:39Z

src/main.ts

+	register(helper: Helper) {
+		helper.yargs.option('c', {
+			alias: 'config',
+			describe: 'Specifies what configuration to test with: browserstack(default), \'testingbot\',  \'saucelabs\', or \'local\'ly.',


do you think that local should be the default?

* Moved to scoped packages * Make local tests the default * Specify capabilities at run time so the project name can be read from package.json

maier49 · 2017-01-18T07:28:35Z

@agubler With the changes merged into todoMVC here, and once these changes are merged into cli-build, this should work with todoMVC.

All your other feedback that I didn't reply to directly should be addressed, and I've tested this with the above changes locally. Testing locally, in saucelabs, and in browserstack all work. One thing to note: if you try to run in saucelabs or browserstack you will see a lot of failures. That's an issue with todoMVC though, as its tests fail in Firefox and IE 10/11, whether they're run via dojo test or grunt test.

codecov-io · 2017-01-18T16:21:35Z

Current coverage is 100% (diff: 100%)

No coverage report found for master at 4e1ee44.

Powered by Codecov. Last update 4e1ee44...8f32841

maier49 added 4 commits January 9, 2017 01:33

Add initial code - need to update tests

9e8b4ed

Add tests and typings

a4c042c

Fix indentation

3957028

Update readme and package.json description

e35235b

dylans requested changes Jan 11, 2017

View reviewed changes

dylans added this to the 2017.01 milestone Jan 11, 2017

maier49 added 2 commits January 11, 2017 12:19

PR cleanup

1f60128

Move node typings to global dependencies so grunt dist works

f066ede

matt-gadd reviewed Jan 11, 2017

View reviewed changes

maier49 added 6 commits January 11, 2017 16:08

Update intern and add testingbot configuration

c84c3ac

Use tabs consistently

2298656

Use selenium tunnel for local intern config

5274c59

Add more saucelabs browser examples

6d7a03f

Add more examples

dd659e1

Update tests for new config options, and add testingbot to config des…

9244306

…cription

dylans approved these changes Jan 12, 2017

View reviewed changes

maier49 added 3 commits January 16, 2017 11:46

Move to scoped packages and pin dojo dependencies

4550eb7

Update intenr files for apps using cli to use scoped packages

6cdf27d

Use path.join in tests

4ab0441

agubler requested changes Jan 16, 2017

View reviewed changes

maier49 added 4 commits January 18, 2017 01:59

Fix various issues

1cc8225

* Moved to scoped packages * Make local tests the default * Specify capabilities at run time so the project name can be read from package.json

Fix path to package.json and order of arguments

fb5c440

Fix order of all the arguments

4a386b3

Add more to readme

14b9197

Remove commented out browsers in environments config

7ccfc39

Remove environments config variable

8f32841

agubler approved these changes Jan 24, 2017

View reviewed changes

agubler merged commit 2c8f8ae into dojo:master Jan 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create test command #1

Create test command #1

maier49 commented Jan 11, 2017

dylans left a comment

dylans Jan 11, 2017

dylans Jan 11, 2017

dylans Jan 11, 2017

dylans Jan 11, 2017

dylans Jan 11, 2017

dylans Jan 11, 2017

maier49 Jan 11, 2017

dylans Jan 11, 2017

dylans Jan 11, 2017

maier49 Jan 11, 2017

dylans Jan 11, 2017

maier49 Jan 11, 2017

dylans Jan 11, 2017

maier49 Jan 11, 2017

dylans Jan 11, 2017

maier49 Jan 12, 2017

matt-gadd Jan 11, 2017

matt-gadd Jan 11, 2017

agubler Jan 16, 2017

agubler Jan 16, 2017

agubler Jan 16, 2017

maier49 Jan 16, 2017 •

edited

Loading

agubler Jan 18, 2017

maier49 Jan 18, 2017

maier49 Jan 18, 2017

agubler Jan 16, 2017

agubler Jan 16, 2017

maier49 commented Jan 18, 2017 •

edited

Loading

codecov-io commented Jan 18, 2017 •

edited

Loading


		Test cases MUST be written using [Intern](https://theintern.github.io) using the Object test interface and Assert assertion interface.

		90% branch coverage MUST be provided for all code submitted to this repository, as reported by istanbul’s combined coverage results for all supported platforms.


		* [Third-party lib one](https//github.com/foo/bar) ([New BSD](http://opensource.org/licenses/BSD-3-Clause))

		© 2004–2016 Dojo Foundation & contributors. [New BSD](http://opensource.org/licenses/BSD-3-Clause) license.

Create test command #1

Create test command #1

Conversation

maier49 commented Jan 11, 2017

dylans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maier49 Jan 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maier49 commented Jan 18, 2017 • edited Loading

codecov-io commented Jan 18, 2017 • edited Loading

Current coverage is 100% (diff: 100%)

maier49 Jan 16, 2017 •

edited

Loading

maier49 commented Jan 18, 2017 •

edited

Loading

codecov-io commented Jan 18, 2017 •

edited

Loading