Fix travis build #302

DorianGrey · 2018-04-11T07:19:16Z

A "work in progress" for now - seems that not all changes made it into the merge with 1.1.1 because we to separate the stuff that should go into react-scripts 2.0 from what is required in 1.x.
Just testing this for the moment and see how far I may get ...

DorianGrey · 2018-04-13T10:11:14Z

@wmonk , things are getting green - yay!
A second look would be good, to take care I did not pick up something unintended. Stuff was picked from CRA's master branch that should not contain things planned for their 2.0 exclusively.

wmonk

Awesome work @DorianGrey 🎉

I have some comments that need to be addressed to get this fully finished, but they shouldn't be very big.

wmonk · 2018-04-13T10:15:42Z

tasks/e2e-installs.sh

-cd "$root_path"/packages/create-react-app
-cli_path=$PWD/`npm pack`
+cd "$temp_app_path"
+npx create-react-app --scripts-version=1.0.17 test-app-version-number


I think --scripts-version here will install react-scripts right?

This test suite is disabled for now.

wmonk · 2018-04-13T10:15:52Z

tasks/e2e-installs.sh

 cd "$temp_app_path"
-create_react_app --scripts-version=0.4.0 test-app-version-number
-cd test-app-version-number
+npx create-react-app --use-npm --scripts-version=1.0.17 test-use-npm-flag


Same as above

wmonk · 2018-04-13T10:16:06Z

tasks/e2e-installs.sh


 cd "$temp_app_path"
-create_react_app --scripts-version=https://registry.npmjs.org/react-scripts/-/react-scripts-0.4.0.tgz test-app-tarball-url
+npx create-react-app --scripts-version=https://registry.npmjs.org/react-scripts/-/react-scripts-1.0.17.tgz test-app-tarball-url


Same as above

wmonk · 2018-04-13T10:16:24Z

tasks/e2e-installs.sh

 exists node_modules/react-scripts
-grep '"version": "0.4.0"' node_modules/react-scripts/package.json
+[ ! -e "yarn.lock" ] && echo "yarn.lock correctly does not exist"
+grep '"version": "1.0.17"' node_modules/react-scripts/package.json


Our version is different now

wmonk · 2018-04-13T10:18:42Z

tasks/e2e-simple.sh


  # Save App.js, we're going to modify it
-  cp src/App.tsx src/App.tsx.bak
+  cp src/App.js src/App.js.bak


This is the bit that shows we're installing the wrong version of react-scripts

wmonk · 2018-04-13T10:19:24Z

tasks/e2e-simple.sh

 # Install the app in a temporary location
 cd $temp_app_path
-create_react_app --scripts-version="$scripts_path" test-app
+npx create-react-app test-app


This is the bit that needs to use the scripts_path variable for --scripts-version.

Done. Due to the new strategy with a local registry, it seems that we only need to provide --scripts-version=react-scripts-ts here.

wmonk · 2018-04-13T10:20:11Z

.travis.yml

 install: true
 script:
 - 'if [ $TEST_SUITE = "simple" ]; then tasks/e2e-simple.sh; fi'
+ - 'if [ $TEST_SUITE = "installs" ]; then tasks/e2e-installs.sh; fi'


I think we can just keep e2e-simple as the others test a lot of stuff from create-react-app we don't care about.

Done.
install is disabled for now.

wmonk · 2018-04-13T10:20:18Z

.travis.yml

-# TODO: reenable after we ship 1.0.
-#    - node_js: 6
-#      env: USE_YARN=yes TEST_SUITE=simple
+    - TEST_SUITE=installs


Can just keep simple here.

Done.
install is disabled for now.

wmonk · 2018-04-13T10:20:49Z

.travis.yml

+    - TEST_SUITE=kitchensink
+matrix:
+  include:
+    - node_js: 0.10


I don't think we want this line, more for testing create-react-app as far as I know.

DorianGrey · 2018-04-13T12:47:51Z

Thanks for the review, I'm looking to fix these issue after my weekend vacation.

DorianGrey · 2018-04-16T07:52:45Z

I've disabled the installs test for now, since this will require a lot of additional work to be done.
I'm still working on the other ones - esp. e2e-simple.sh was changed a lot, now using a local registry instead of local packing. Still experimenting which parts have to reverted to the previous strategy.

wmonk · 2018-04-17T09:02:02Z

Awesome work @DorianGrey - is this ready to be merged?

DorianGrey · 2018-04-17T09:17:09Z

Suppose, yes.

We might have to deal with e2e-installs.sh, but I don't think it's that important for now. I've just disabled the test suite and left a particular comment that additional work has to be done before activating this again.

* master: Stick @types/node to 9.6.7 to avoid problems with 10.0.0 (wmonk#315) Fix travis build (wmonk#302) v2.15.1 Update README For 2.15.1 fix(modules): remove duplicate mjs from jest config

DorianGrey force-pushed the travis-build-fix-attempt branch 5 times, most recently from 2c624b9 to e9d87b1 Compare April 13, 2018 09:37

Attempt to fix travis build

544ae2b

DorianGrey force-pushed the travis-build-fix-attempt branch from e9d87b1 to 544ae2b Compare April 13, 2018 09:39

DorianGrey requested a review from wmonk April 13, 2018 10:11

wmonk suggested changes Apr 13, 2018

View reviewed changes

wmonk mentioned this pull request Apr 13, 2018

create-react-app target broken #304

Closed

react-scripts-ts specific adoptions

169a401

DorianGrey force-pushed the travis-build-fix-attempt branch from 0a1f4ea to 169a401 Compare April 17, 2018 07:35

wmonk approved these changes Apr 17, 2018

View reviewed changes

DorianGrey changed the title ~~[W.I.P.] Attempt to fix travis build~~ Fix travis build Apr 27, 2018

DorianGrey merged commit 97d5561 into wmonk:master Apr 27, 2018

Fix travis build #302

Fix travis build #302

Uh oh!

Conversation

DorianGrey commented Apr 11, 2018

Uh oh!

DorianGrey commented Apr 13, 2018

Uh oh!

wmonk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DorianGrey commented Apr 13, 2018

Uh oh!

DorianGrey commented Apr 16, 2018

Uh oh!

wmonk commented Apr 17, 2018

Uh oh!

DorianGrey commented Apr 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants