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

feat(config): Add configuration for adding javascript version. #1983

Conversation

Moumi
Copy link
Contributor

@Moumi Moumi commented Mar 10, 2016

Add the configuration to add a javascript version tag to the loaded scripts. Only applied when the Firefox browser is run.

Closes #1719.

@Moumi
Copy link
Contributor Author

Moumi commented Mar 10, 2016

While looking at the Travis CI results, it probably is the fact that Chrome is not available on that server. Should I change this to PhantomJS or is it no issue?

/cc @dignifiedquire

Firefox
"""

Scenario: Execute a test in Chrome with version, without JavaScript tag
Copy link
Member

Choose a reason for hiding this comment

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

This should be tagged with @not-jenkins so it's not run on travis

@Moumi Moumi force-pushed the feature-javascript-version-tag branch from 19a8ac7 to bc7fd1d Compare March 10, 2016 16:32
@Moumi
Copy link
Contributor Author

Moumi commented Mar 10, 2016

I have successfully made the change that was suggested. It seems that it made the difference and the changes I made actually work now. However, I got the following error (on Jenkins) for the first time ever:

launchers/process.js flow start -> timeout -> 3xrestart -> failure:
     Uncaught AssertionError: expected spy to have been called with arguments /usr/bin/browser, ["http://localhost/?id=fake-id"]
    at test/unit/launchers/process.spec.js:232:42
    at [object Object]._onTimeout (node_modules/lodash/index.js:1773:43)

Not sure why it gives an error there, but it has nothing to do with my change. If it is a common, or even new issue, I will create an issue for it 😉

@dignifiedquire
Copy link
Member

Yeah you can ignore that one, no idea why it fails sometimes (was never able to reproduce it locally :( )

@Moumi
Copy link
Contributor Author

Moumi commented Mar 10, 2016

Ah oke, good. Hope to have helped with this PR ;)

@dignifiedquire
Copy link
Member

Thanks, looks good but needs some documentation in the config section of the docs.

@Moumi
Copy link
Contributor Author

Moumi commented Mar 19, 2016

I will write some documentation for it tomorrow. Expect it to have it tomorrow evening.

@dignifiedquire
Copy link
Member

Thanks @Moumi

@Moumi Moumi force-pushed the feature-javascript-version-tag branch from bc7fd1d to c25ed00 Compare March 20, 2016 20:39
@Moumi
Copy link
Contributor Author

Moumi commented Mar 20, 2016

Finished it @dignifiedquire ;)


**Description:** The JavaScript version to use in the FireFox browser.

If `> 0`, Karma will add a JavaScript version tag to the included .js-files.
Copy link
Member

Choose a reason for hiding this comment

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

Better: "included JavaScript files"

Add the configuration to add a javascript version tag to the loaded scripts. Only applied when the Firefox browser is run.

Closes karma-runner#1719.
@Moumi Moumi force-pushed the feature-javascript-version-tag branch from c25ed00 to 0239c75 Compare March 21, 2016 00:32
@Moumi
Copy link
Contributor Author

Moumi commented Mar 21, 2016

Applied your comments 😉

@dignifiedquire
Copy link
Member

Thanks :octocat:

dignifiedquire added a commit that referenced this pull request Mar 21, 2016
…-tag

feat(config): Add configuration for adding javascript version.
@dignifiedquire dignifiedquire merged commit f4c8c04 into karma-runner:master Mar 21, 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.

2 participants