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

CI Test work #1212

Merged
merged 2 commits into from Apr 29, 2015

Conversation

Projects
None yet
4 participants
@samccone
Member

samccone commented Mar 17, 2015

Master still runs deploy but all other branches and PR's will run the tests on the changes.

✨ πŸ’ƒ ✨

I anticipate a bit of friction when we flip this on.. just with the sauce keys and such but, everything works on my fork πŸ‘

  • First we add the real remote and fetch the contents
  • then we diff the current branch from masters contents
  • then we get the examples folders.. for instance

examples/backbone/....
examples/angular/.....

then I awk out the middle name, which happens to be used with our test
suite to run each test. From there I use xargs to invoke the tests
running over a subset of all of the tests.

npm run test -- --framework=backbone --framework=angular
--framework=dogescript

commit 1 allows us to do this

npm run test -- --framework=jquery --framework=backbone --grep 'should trim entered text'


Fixes #946

@ColinEberhardt

This comment has been minimized.

Show comment
Hide comment
@ColinEberhardt

ColinEberhardt Mar 17, 2015

Member

Wow, thanks @samccone - that is absolutely awesome, very happy to see this implemented :-)

One area that I think will cause friction is the implementations that do not fully adhere to the HTML specification. This is why added a laxMode concept:

https://github.com/tastejs/todomvc/tree/master/browser-tests#lax-mode

The frameworks that require the laxMode setting isn't documented anywhere. Perhaps this should be recorded within allTests.js alongside the other metadata.

I think the tests code needs a bit of a tidy-up, but having them as part of the CI should ensure they are run and maintained. I was hoping to find some time to give them a bit of attention in the near future.

Anyhow, bug thumbs-up from me πŸ‘

Member

ColinEberhardt commented Mar 17, 2015

Wow, thanks @samccone - that is absolutely awesome, very happy to see this implemented :-)

One area that I think will cause friction is the implementations that do not fully adhere to the HTML specification. This is why added a laxMode concept:

https://github.com/tastejs/todomvc/tree/master/browser-tests#lax-mode

The frameworks that require the laxMode setting isn't documented anywhere. Perhaps this should be recorded within allTests.js alongside the other metadata.

I think the tests code needs a bit of a tidy-up, but having them as part of the CI should ensure they are run and maintained. I was hoping to find some time to give them a bit of attention in the near future.

Anyhow, bug thumbs-up from me πŸ‘

@addyosmani

This comment has been minimized.

Show comment
Hide comment
@addyosmani

addyosmani Mar 19, 2015

Member

The frameworks that require the laxMode setting isn't documented anywhere. Perhaps this should be recorded within allTests.js alongside the other metadata.

πŸ‘

Anyhow, bug thumbs-up from me

Same! This is terrific work.

Member

addyosmani commented Mar 19, 2015

The frameworks that require the laxMode setting isn't documented anywhere. Perhaps this should be recorded within allTests.js alongside the other metadata.

πŸ‘

Anyhow, bug thumbs-up from me

Same! This is terrific work.

Show outdated Hide outdated browser-tests/test.js
if (process.env.SAUCE_USERNAME != undefined) {
browser = new webdriver.Builder()
.usingServer('http://'+ process.env.SAUCE_USERNAME+':'+process.env.SAUCE_ACCESS_KEY

This comment has been minimized.

@arthurvr

arthurvr Mar 20, 2015

Member

Spaces around operators.

@arthurvr

arthurvr Mar 20, 2015

Member

Spaces around operators.

Show outdated Hide outdated browser-tests/test.js
.withCapabilities({browserName : browserName})
.build();
if (process.env.SAUCE_USERNAME != undefined) {

This comment has been minimized.

@arthurvr

arthurvr Mar 20, 2015

Member

Jshint complaint in here :D Use triple equal.

@arthurvr

arthurvr Mar 20, 2015

Member

Jshint complaint in here :D Use triple equal.

Show outdated Hide outdated browser-tests/test.js
browserName : browserName
})
.build();
}

This comment has been minimized.

@arthurvr

arthurvr Mar 20, 2015

Member

Actually there are 9 jscs errors on this piece of js

@arthurvr

arthurvr Mar 20, 2015

Member

Actually there are 9 jscs errors on this piece of js

This comment has been minimized.

@samccone

samccone Mar 25, 2015

Member

i only had 2... fixed though

@samccone

samccone Mar 25, 2015

Member

i only had 2... fixed though

@arthurvr

This comment has been minimized.

Show comment
Hide comment
@arthurvr

arthurvr Mar 20, 2015

Member

Great work Sam!

Member

arthurvr commented Mar 20, 2015

Great work Sam!

Show outdated Hide outdated .travis.yml
env:
global:
- secure: K1WJRK2WTcc8+ZVGfwgOR8dBSM3VqJXNzcFpRPbEd48kVikx6nKYhrSffyl8a5QM5+Z7UP1TrmRAmOwR09Qp7Lh2Ch7vE9ooK9gk3XsoUP454aWm35NzjqagyajzGe7disCf6nWxgPZ0lgNkCG18aBfyh9FlUryzGzRyiSqeVeQ=
- secure: Or0NreVqcSIE6lbmSe3TmeXlm/++hXZAN9LVozvwlRLrE2jevIYRWD6arep9RTACtiQOPLrLrMh1/KDlIMgSfdrSa/w72J+YchSrYm+d06p4BcHPZEDnooiktTx4e9nL/5MfxIUYNOF0iT+wReRxId0INukqIUvjKB4F/X7MgmE=

This comment has been minimized.

@arthurvr

arthurvr Mar 20, 2015

Member

People might wonder what is included in these. Might be worth it to include some comments similar to the ones on lines 16 till 27.

@arthurvr

arthurvr Mar 20, 2015

Member

People might wonder what is included in these. Might be worth it to include some comments similar to the ones on lines 16 till 27.

Show outdated Hide outdated browser-tests/allTests.js
@@ -63,7 +63,9 @@ list = list.filter(function (framework) {
// if a specific framework has been named, just run this one
if (argv.framework) {
list = list.filter(function (framework) {
return framework.name === argv.framework;
return [].concat(argv.framework).some(function(f) {

This comment has been minimized.

@arthurvr

arthurvr Mar 25, 2015

Member

function(f) { -> function (f) {

@arthurvr

arthurvr Mar 25, 2015

Member

function(f) { -> function (f) {

This comment has been minimized.

This comment has been minimized.

All users to run multiple tests at once via CLI
this enables users to do the following

npm run test -- --framework=jquery --framework=backbone --grep 'should trim entered text'
@samccone

This comment has been minimized.

Show comment
Hide comment
@samccone

samccone Mar 25, 2015

Member

updated ✨

Member

samccone commented Mar 25, 2015

updated ✨

Show outdated Hide outdated travis-runner.sh
git push https://${GH_OAUTH_TOKEN}@github.com/${GH_OWNER}/${GH_PROJECT_NAME} HEAD:gh-pages > /dev/null 2>&1
else
git remote add real https://github.com/tastejs/todomvc.git && \
git fetch real && \

This comment has been minimized.

@arthurvr

arthurvr Mar 25, 2015

Member

Real seems like a confusing name.

@arthurvr

arthurvr Mar 25, 2015

Member

Real seems like a confusing name.

This comment has been minimized.

@samccone

samccone Mar 25, 2015

Member

thoughts on a better name?

@samccone

samccone Mar 25, 2015

Member

thoughts on a better name?

This comment has been minimized.

@arthurvr

arthurvr Mar 25, 2015

Member

Just some brain farts but what about current or stable or so

@arthurvr

arthurvr Mar 25, 2015

Member

Just some brain farts but what about current or stable or so

This comment has been minimized.

@samccone

samccone Mar 25, 2015

Member

sure current is fine, will update

@samccone

samccone Mar 25, 2015

Member

sure current is fine, will update

Show outdated Hide outdated browser-tests/test.js
if (process.env.SAUCE_USERNAME !== undefined) {
browser = new webdriver.Builder()
.usingServer(
'http://' +

This comment has been minimized.

@arthurvr

arthurvr Mar 27, 2015

Member

Two tabs seems weird over here.

@arthurvr

arthurvr Mar 27, 2015

Member

Two tabs seems weird over here.

This comment has been minimized.

@samccone

samccone Mar 27, 2015

Member

i agree, on it

@samccone

samccone Mar 27, 2015

Member

i agree, on it

This comment has been minimized.

@arthurvr

arthurvr Mar 29, 2015

Member

now it's totally not indented. That's even more weird. You just need one tab.

@arthurvr

arthurvr Mar 29, 2015

Member

now it's totally not indented. That's even more weird. You just need one tab.

This comment has been minimized.

@samccone

samccone Mar 29, 2015

Member

πŸ‘ fixed

@samccone

samccone Mar 29, 2015

Member

πŸ‘ fixed

@samccone

This comment has been minimized.

Show comment
Hide comment
@samccone

samccone Mar 29, 2015

Member

did anyone else want to take a look at this?

Member

samccone commented Mar 29, 2015

did anyone else want to take a look at this?

Run tests over a subset of tests based on changes
* First we add the real remote and fetch the contents
* then we diff the current branch from masters contents
* then we get the examples folders.. for instance

examples/backbone/....
examples/angular/.....

then I awk out the middle name, which happens to be used with our test
suite to run each test. From there I use xargs to invoke the tests
running over a subset of all of the tests.

npm run test -- --framework=backbone --framework=angular
--framework=dogescript
@samccone

This comment has been minimized.

Show comment
Hide comment
@samccone

samccone Mar 30, 2015

Member

@arthurvr your style comments have been addressed :)

Member

samccone commented Mar 30, 2015

@arthurvr your style comments have been addressed :)

@samccone

This comment has been minimized.

Show comment
Hide comment
Member

samccone commented Apr 1, 2015

@samccone

This comment has been minimized.

Show comment
Hide comment
Member

samccone commented Apr 6, 2015

@arthurvr

This comment has been minimized.

Show comment
Hide comment
@arthurvr

arthurvr Apr 7, 2015

Member

If nobody complains let's land this one πŸ‘

Member

arthurvr commented Apr 7, 2015

If nobody complains let's land this one πŸ‘

arthurvr added a commit that referenced this pull request Apr 29, 2015

@arthurvr arthurvr merged commit 1368764 into tastejs:master Apr 29, 2015

@samccone samccone deleted the samccone:sjs/test-work branch Apr 29, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment