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

Added JS Jasmine tests to Travis CI #9013

Merged
merged 18 commits into from
Mar 31, 2017
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,22 @@ env:
- TEST_SUITE=integration INTEGRATION_INDEX=2
- TEST_SUITE=integration INTEGRATION_INDEX=3
- TEST_SUITE=static
- TEST_SUITE=js
cache:
apt: true
directories: $HOME/.composer/cache
directories:
- $HOME/.composer/cache
- $HOME/.nvm
matrix:
exclude:
- php: 5.6.29
env: TEST_SUITE=static
- php: 5.6.29
env: TEST_SUITE=js
before_install: ./dev/travis/before_install.sh
install: composer install --no-interaction --prefer-dist
before_script: ./dev/travis/before_script.sh
script:
- test $TEST_SUITE = "static" && TEST_FILTER='--filter "Magento\\Test\\Php\\LiveCodeTest"' || true
- phpunit -c dev/tests/$TEST_SUITE $TEST_FILTER
- if [ $TEST_SUITE != "js" ]; then phpunit -c dev/tests/$TEST_SUITE $TEST_FILTER; fi
- if [ $TEST_SUITE == "js" ]; then grunt spec; fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at https://travis-ci.org/magento/magento2/jobs/215020317 - is there some less verbose mode? Display only failed tests maybe...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea, for now I just focuses on make this test running, then we can think about some adjustments / improvements.

PS. I really hate Grunt, so if I don't have to modify this, I will be more than happy to leave it as it is 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I didn't use it as well, and didn't google for a solution just in case you know the answer.

It can wait until it become a problem "output is too big" some day (hopefully never).

16 changes: 16 additions & 0 deletions dev/travis/before_script.sh
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,20 @@ case $TEST_SUITE in

cd ../../..
;;
js)
curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.33.1/install.sh | bash
export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh" # This loads nvm
nvm install --lts
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it would be more robust to specify some concrete version, like in https://github.com/BanzaiMan/travis_production_test/blob/9c02aef/.travis.yml#L15, so that we see in environment variable which version we test against

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if we really need to care about precise versioning, b/c in Oct 2017 we should be ready to use v7 without any issues and necessary updates in code base.

Node.js LTS plan is precisely described here - https://github.com/nodejs/LTS

Copy link
Contributor

Choose a reason for hiding this comment

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

If I understood the doc right v7 is not LTS and will not be used in such code.

The point is that I don't see any value in dynamic Node.js version, it can only bring us some headache when the LTS version will switch. Explicit is better than implicit.

As soon as v8 stable is released we can change value in our config if the tests still pass. Current version is like specifying php: stable version instead of concrete value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, v8, not v7.
LTS means more than stable, b/c stable v8 will be released in April 2017, but will be marked as LTS in Oct 2017. It's a different approach than PHP takes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it guaranteed that using v6 for tests before, we will not have any issues under v8 when it become LTS? What's BC police?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OFC there are BC between major releases, but major becomes LTS after half year and most of the packages get updates during this period, so migration is in most of the cases are seamless.

Copy link
Contributor

Choose a reason for hiding this comment

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

It does not mean all our tests will work without changes.

Just answer the simple question - what is the value of NOT specifying a concrete version but using LTS instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

By the way, package.json.sample bundled with Magento also has version requirement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are always up to date, don't have to follow releases schedule etc. When new LTS version will be up, your tests will start using it.

But to just quit discussion, I'll change it to 6 and this will be good enough till next LTS.

Docs about engines option from package.json

this field is advisory only will produce warnings when your package is installed as a dependency.

So in our case, we can simply remove this param.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

When new LTS version will be up, your tests will start using it.

Yeah, that's a scary part as to me, as put our builds a bit out of control.

nvm use --lts

cp package.json.sample package.json
cp Gruntfile.js.sample Gruntfile.js
npm install -g yarn
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really viable to install yarn just to install grunt-cli?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not only grunt-cli, all dependencies defined in package.json are downloaded using yarn in line 90 (yarn is replacement for npm install).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for clarification.

yarn global add grunt-cli
yarn

cp dev/travis/js/config.php app/etc
php bin/magento setup:static-content:deploy -f
;;
esac
Loading