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

Changes to use karma and adds firefox headless #1087

Conversation

deiga
Copy link
Contributor

@deiga deiga commented Oct 11, 2017

Addressses #1045

@deiga
Copy link
Contributor Author

deiga commented Oct 11, 2017

@kamilogorek Not sure why SAUCE variables are missing, pls halp? :)

@kamilogorek
Copy link
Contributor

@deiga Travis doesn't expose ENV vars to PRs coming from external contributors (non-org accounts), as they could be easily extracted and used on other projects. We're working on getting around this soon.

I'll review your PR today, thanks! :)

Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

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

I tried to run those changes locally, and I'm not able to trigger any tests.

npm run test:unit

give me this output

START:
11 10 2017 15:07:13.565:WARN [watcher]: Pattern "/Users/kamilogorek/Projects/sentry/raven-js/build/raven.test.js" does not match any file.
11 10 2017 15:07:13.619:INFO [karma]: Karma v1.7.1 server started at http://0.0.0.0:9876/
11 10 2017 15:07:13.620:INFO [launcher]: Launching browsers ChromeHeadless, FirefoxHeadless with unlimited concurrency
11 10 2017 15:07:13.626:INFO [launcher]: Starting browser ChromeHeadless
11 10 2017 15:07:13.634:INFO [launcher]: Starting browser Firefox
11 10 2017 15:07:14.262:INFO [HeadlessChrome 0.0.0 (Mac OS X 10.12.6)]: Connected on socket iEPdymhObeJm1HLDAAAA with id 33628822
11 10 2017 15:07:16.206:INFO [Firefox 56.0.0 (Mac OS X 10.12.0)]: Connected on socket NL4Ypu659I4dCYZ2AAAB with id 33702021

Finished in 0.018 secs / 0 secs @ 15:07:16 GMT+0200 (CEST)

SUMMARY:
✔ 0 tests completed
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test:karma:unit: `karma start karma.config.js "--single-run"`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] test:karma:unit script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kamilogorek/.npm/_logs/2017-10-11T13_07_16_620Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test:unit: `npm run test:karma:unit -- --single-run`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] test:unit script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/kamilogorek/.npm/_logs/2017-10-11T13_07_16_669Z-debug.log

karma.config.js Outdated
}
}
});
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Those two config files are in 90% the same. Can we move common config to something like karma-base.config.js and then import it in karma-integration.config.js and karma-unit.config.js?

var runner = mocha.run();
var failedTests = [];

var flattenTitles = function (test) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This code is required in order for tests to work on SauceLabs.

@deiga
Copy link
Contributor Author

deiga commented Oct 11, 2017

@kamilogorek Could you use https://docs.travis-ci.com/user/encryption-keys/ to add the SAUCE env variables?

You need to run npm test, since the tests are still dependent on build built first :)

I'll work on those comments tonight

@deiga
Copy link
Contributor Author

deiga commented Oct 11, 2017

@kamilogorek I addressed the issues with the index and saucelabs by changing to use karma for that too.

@kamilogorek
Copy link
Contributor

@kamilogorek Could you use https://docs.travis-ci.com/user/encryption-keys/ to add the SAUCE env variables?

Not really, it will still fail. Per docs:

Please note that encrypted environment variables are not available for pull requests from forks.

You need to run npm test, since the tests are still dependent on build built first :)

I got bitten by my own setup... 🤦‍♂️

Thanks! Will reverify it Today :)

@deiga
Copy link
Contributor Author

deiga commented Oct 12, 2017

Oh, yeah. That's a bummer. Does SauceLabs have any kind of ACL, where you could have "public" credentials which only travis could use to send data?

@kamilogorek
Copy link
Contributor

Not that I know of. We'll most likely write a bot that'd run those tests for us instead.

@kamilogorek
Copy link
Contributor

kamilogorek commented Oct 12, 2017

@deiga can you grant me write access your fork? I rebased your changes and added some changes that would like to include here, as I don't want to open a new PR :)

https://github.com/deiga/raven-js/settings/collaboration

@deiga
Copy link
Contributor Author

deiga commented Oct 12, 2017

@kamilogorek done

@kamilogorek kamilogorek force-pushed the switch-to-karma-and-integrate-headless-firefox branch from 2e294cf to d8c8cae Compare October 13, 2017 08:30
@kamilogorek kamilogorek merged commit b05c2d0 into getsentry:master Oct 13, 2017
@kamilogorek
Copy link
Contributor

@deiga merged :) You can take a look at my latests commits to see what had to be changed in order to run everything on SauceLabs

untitled-1

@deiga deiga deleted the switch-to-karma-and-integrate-headless-firefox branch October 13, 2017 09:41
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.

2 participants