Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

Update test commands #1245

Merged
merged 5 commits into from
Aug 29, 2018
Merged

Update test commands #1245

merged 5 commits into from
Aug 29, 2018

Conversation

happyhj
Copy link
Contributor

@happyhj happyhj commented Aug 23, 2018

Description

Currently, running $ yarn test --browsers=Chrome --grep='multinomial' don't filters tests.

The syntax to use to pass arguments to the package script running with yarn (run) has changed.

Because of this, the arguments that are passed to the yarn test which is posted in DEVELOPMENT.md are not applied to the karma start command.

So I updated the command to match the new syntax.

Please refer to the issue below
yarnpkg/yarn#1667


For repository owners only:

Please remember to apply all applicable tags to your pull request.
Tags: FEATURE, BREAKING, BUG, PERF, DEV, DOC, SECURITY

For more info see: https://github.com/tensorflow/tfjs/blob/master/DEVELOPMENT.md


This change is Reviewable

@nsthorat
Copy link
Contributor

Which version of yarn are you using?

With yarn -v I get 1.5.1.

When I run this I get the following message:

warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.

In the issue that you linked, the user is using yarn version 0.16.1.

@tafsiri
Copy link
Contributor

tafsiri commented Aug 29, 2018

One fix could be to specify a minimum yarn version in the 'engines' entry of our package.json https://yarnpkg.com/en/docs/package-json#toc-engines. We could try >= 1.4.0 as a start.

* And restore test command
@happyhj
Copy link
Contributor Author

happyhj commented Aug 29, 2018

@nsthorat You're right. My yarn version was outdated. (v0.16.1) I updated yarn to v1.5.1 and it works like a magic. So I got the test commands back.

@tafsiri I think that is good idea to specify minimum yarn version. So I done that on my last commit.

@happyhj
Copy link
Contributor Author

happyhj commented Aug 29, 2018

The travis CI's yarn version is v1.3.2. So I lowered the minimum yarn version to fix the error on CI.

@nsthorat nsthorat merged commit 01fa393 into tensorflow:master Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants