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

html runner now supports specifing .html tests in url #35

Closed
wants to merge 2 commits into from

Conversation

adros
Copy link
Contributor

@adros adros commented Feb 3, 2016

This PR modifies doh html runner, so it supports opening tests written as .html, e.g.:

util/doh/runner.html?test=dijit/tests/Menu.html,dojox/lang/tests/array,dijit/tests/Destroyable.html

@dylans dylans added this to the 1.12 milestone Feb 3, 2016
@dylans dylans self-assigned this Feb 3, 2016
@wkeese
Copy link
Member

wkeese commented Mar 11, 2016

Looks reasonable to me.

Copy link
Contributor

@jacobroufa jacobroufa left a comment

Choose a reason for hiding this comment

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

Aside from the variable declarations on L195-196, this looks good.

domReady(function(){
doh._fixHeight();
doh.breakOnError= breakOnError;
for (var amdTests = [], i = 0, l = test.length; i < l; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

amdTests should be declared at the top of the function scope, as it is used later, outside of this block.

doh._fixHeight();
doh.breakOnError= breakOnError;
for (var amdTests = [], i = 0, l = test.length; i < l; i++) {
var module = test[i];
Copy link
Contributor

Choose a reason for hiding this comment

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

module should be declared at the top of the function scope, so it is not redeclared every time inside of this loop.

@adros
Copy link
Contributor Author

adros commented Oct 28, 2016

I have fixed the variable declaration.

(no idea why my commit was marked with CLA: Error (Autor has not signed CLA), because it was commited with same account as the first one and I have signed CLA years ago)

@agubler
Copy link
Member

agubler commented Oct 28, 2016

Hi @adros, since becoming the JS Foundation I think everyone needs to resign the CLA.

@dylans
Copy link
Member

dylans commented Oct 28, 2016

Thanks @adros , this has landed in master (2dd35b5) and will be in the 1.12 release in a few week.

@dylans dylans closed this Oct 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants