Skip to content

Conversation

@watadarkstar
Copy link
Contributor

@watadarkstar watadarkstar commented Nov 25, 2017

Adds bundle linting and tests to the release script. Closes: #11660

Test check list:

  • Ran yarn
  • Passed all tests with yarn test, yarn test-prod
  • Ran yarn prettier
  • Ran yarn lint
  • Ran yarn flow
  • Completed CLA
  • Ran yarn build and then yarn lint-build (new script I added)
  • Ran ./scripts/release/build.js -v 16.2.0

 - add yarn lint-build
- use yarn lint-build in circle ci build.sh
- add yarn lint-build, yarn test-prod, yarn test-build, and yarn test-build-prod to the realse script
@watadarkstar
Copy link
Contributor Author

@gaearon Hi Dan!

Ran into a few issues when trying to run node ./scripts/release/build.js -v 16.2.0 to test my changes.

Firstly I got:

ERROR  Missing CircleCI API token

I created an account on CircleCI and did export CIRCLE_CI_API_TOKEN=<your-token-here>.

Then I got:

 ERROR  Command failed: npm whoami

I created an account on npmjs and did npm adduser

Then I got:

ERROR  Insufficient NPM permissions

NPM user adriancarolli is not an owner for:
react-reconciler, react-call-return, react-test-renderer, react-dom, react-art, react

Please contact a React team member to be added to the above project(s).

This is where I am stumped. Any suggestions?

@gaearon
Copy link
Collaborator

gaearon commented Nov 25, 2017

Sorry I forgot about those checks. For testing, you can comment these steps out in release/build.js. It's pretty readable: the list of all tasks is at the bottom, with await before each.

@watadarkstar
Copy link
Contributor Author

watadarkstar commented Nov 25, 2017

@gaearon Thanks.

Thoughts:

  • I think we will need to move await buildArtifacts(params) to above await runAutomatedTests(params) since those tests are now running yarn lint-build they depend on the bundle being built.
  • Alternatively, we create a new function that runs bundle specific tests only i.e. runAutomatedBundleTests(params) and call it after await buildArtifacts(params).
  • And or we add something like await checkBundleIsPresent(params) before runAutomatedTests(params)

EDIT: i.e. I get an error when the bundle isn't present:

$ node ./scripts/release/build.js -v 16.2.0                                                                                                            
✓ Running ESLint
 ERROR  Lint bundle failed

yarn run v1.3.2
$ node ./scripts/rollup/validate/index
Checking if files exist in ./build/facebook-www/*.js...

Based on #10620 I am understanding that one would run yarn build before node ./scripts/release/build.js -v 16.2.0 gets called and therefore a bundle would be present, but I'm not sure because isn't node ./scripts/release/build.js -v 16.2.0 suppose to automate the process? Nonetheless, the fact that runAutomatedTests now depends on the bundle being present, node ./scripts/release/build.js now breaks when the bundle isn't present. Dan, let me know how you would like me to proceed.

@gaearon
Copy link
Collaborator

gaearon commented Nov 26, 2017

Alternatively, we create a new function that runs bundle specific tests only i.e. runAutomatedBundleTests(params) and call it after await buildArtifacts(params).

This sounds good.

- Moved the runYarnTask into utils since its being used two files now
- Uncomment out checks I mistakenly committed
Mistakenly commited by release script
@watadarkstar
Copy link
Contributor Author

watadarkstar commented Nov 26, 2017

@gaearon This should be good to be reviewed, I tested ./scripts/release/build.js -v 16.2.0 and it works with my changes.

@gaearon gaearon merged commit 53ef71b into facebook:master Nov 26, 2017
@gaearon
Copy link
Collaborator

gaearon commented Nov 26, 2017

This is great. Thank you!

HeroProtagonist pushed a commit to HeroProtagonist/react that referenced this pull request Nov 26, 2017
* Add bundle linting and tests to the release script

 - add yarn lint-build
- use yarn lint-build in circle ci build.sh
- add yarn lint-build, yarn test-prod, yarn test-build, and yarn test-build-prod to the realse script

* Improve readability of release test messages

* Run prettier

* Updating package versions for release 16.2.0

* Seperate bundle specific tests

- Moved the runYarnTask into utils since its being used two files now
- Uncomment out checks I mistakenly committed

* Revert a bunch of version bump changes

Mistakenly commited by release script

* .js for consistency
raphamorim pushed a commit to raphamorim/react that referenced this pull request Nov 27, 2017
* Add bundle linting and tests to the release script

 - add yarn lint-build
- use yarn lint-build in circle ci build.sh
- add yarn lint-build, yarn test-prod, yarn test-build, and yarn test-build-prod to the realse script

* Improve readability of release test messages

* Run prettier

* Updating package versions for release 16.2.0

* Seperate bundle specific tests

- Moved the runYarnTask into utils since its being used two files now
- Uncomment out checks I mistakenly committed

* Revert a bunch of version bump changes

Mistakenly commited by release script

* .js for consistency
Ethan-Arrowood pushed a commit to Ethan-Arrowood/react that referenced this pull request Dec 8, 2017
* Add bundle linting and tests to the release script

 - add yarn lint-build
- use yarn lint-build in circle ci build.sh
- add yarn lint-build, yarn test-prod, yarn test-build, and yarn test-build-prod to the realse script

* Improve readability of release test messages

* Run prettier

* Updating package versions for release 16.2.0

* Seperate bundle specific tests

- Moved the runYarnTask into utils since its being used two files now
- Uncomment out checks I mistakenly committed

* Revert a bunch of version bump changes

Mistakenly commited by release script

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants