Use verdaccio to test create-project #877

eliperelman · 2018-05-17T14:36:20Z

Fixes #702.

edmorley

Nice PR! The Travis end-to-end times are actually quicker than master, even with the added overhead (though guess since smaller matrix). Definitely a better tradeoff IMO.

edmorley · 2018-05-17T15:23:51Z

package.json

    "validate:eslintrc:root": "eslint --no-eslintrc --print-config . -c ./.eslintrc.js > /dev/null",
    "validate:eslintrc:eslint": "eslint --no-eslintrc --print-config . -c ./packages/eslint/eslintrc.js > /dev/null",
    "validate:eslintrc:airbnb": "eslint --no-eslintrc --print-config . -c ./packages/airbnb/eslintrc.js > /dev/null",
    "validate:eslintrc:airbnb-base": "eslint --no-eslintrc --print-config . -c ./packages/airbnb-base/eslintrc.js > /dev/null",
    "validate:eslintrc:standardjs": "eslint --no-eslintrc --print-config . -c ./packages/standardjs/eslintrc.js > /dev/null",
    "validate:eslintrc": "yarn validate:eslintrc:eslint && yarn validate:eslintrc:airbnb-base && yarn validate:eslintrc:airbnb && yarn validate:eslintrc:standardjs && yarn validate:eslintrc:root",
-    "version": "yarn changelog --package && git add CHANGELOG.md"
+    "version": "([[ -n \"$NEUTRINO_RELEASE\" ]] && yarn changelog --package && git add CHANGELOG.md) || exit 0"


bash 4 and above has a new -v conditional for "is this variable set" that doesn't need the dollar sign or quotes - that could make this easier to read. Also the current || exit 0 will hide any real errors that occur during the changelog command.

Something like this should work? (Rather than using [[ ! -v NEUTRINO_RELEASE ]], I've inverted the conditional name/purpose to avoid the negation)

"version": "[[ -v SKIP_CHANGELOG ]] || yarn changelog --package && git add CHANGELOG.md"

...or using test (I can't decide which looks cleaner):

"version": "test -v SKIP_CHANGELOG || yarn changelog --package && git add CHANGELOG.md"

edmorley · 2018-05-17T15:38:57Z

package.json

    "release:preview": "yarn build && lerna publish --force-publish=* --skip-git --skip-npm",
-    "test": "yarn test:all --no-verbose --match '!*@neutrinojs*'",
-    "test:all": "nyc --reporter lcov ava --verbose --fail-fast packages/*/test",
+    "test": "ava --fail-fast packages/*/test '!packages/create-project/test'",


The !packages/create-project/test doesn't seem to work for me locally (the create project test still tries to run), however it might just be a Windows-only issue. (Happy to ignore so long as it works for you / on Travis)

I wonder if it's because it should be double-quoted?

edmorley · 2018-05-17T15:43:37Z

scripts/test-create-project-ci.sh

+yarn verdaccio --config verdaccio.yml & sleep 10;
+yarn config set registry "http://localhost:4873";
+npm config set registry "http://localhost:4873";
+scripts/npm-adduser.js;


I wonder if it's possible to use anonymous publish mode to avoid the need for the npm-adduser.js step and the npm dependency?
https://www.verdaccio.org/docs/en/authentification.html#anonymous-publish

Oooooooo lemme try.

Actually, I remember trying this but was unable to get it to work in the past. I'll try it again.

edmorley · 2018-05-17T15:48:12Z

scripts/test-create-project-ci.sh

+  --skip-npm \
+  --registry http://localhost:4873/ \
+  --yes \
+  --repo-version $(node_modules/.bin/semver -i patch $(npm view neutrino version));


Could we use --cd-version here to avoid the external request and dep on semver?
https://github.com/lerna/lerna#--cd-version

If not, perhaps let's use yarn for the version lookup instead of npm (yarn info neutrino version).

edmorley · 2018-05-17T15:48:30Z

scripts/test-create-project-ci.sh

+  --registry http://localhost:4873/ \
+  --yes \
+  --repo-version $(node_modules/.bin/semver -i patch $(npm view neutrino version));
+yarn lerna exec npm publish --registry http://localhost:4873/;


Could we try using yarn instead of npm here?

edmorley · 2018-05-17T15:55:22Z

packages/create-project/test/cli_test.js


-    test.serial(preset, async t => {
+  if (tester) {


There are only 3 lines different between the true and false case here - what do you think about flattening this conditional and inlining them, to reduce duplication/potential for deviation and clarify what's different?

Good point.

edmorley · 2018-05-17T15:56:14Z

scripts/test-create-project-ci.sh

+  --yes \
+  --repo-version $(node_modules/.bin/semver -i patch $(npm view neutrino version));
+yarn lerna exec npm publish --registry http://localhost:4873/;
+yarn test:create-project;


Missing trailing newline

edmorley · 2018-05-17T15:57:25Z

scripts/test-create-project-ci.sh

@@ -0,0 +1,15 @@
+#! /usr/bin/env bash


Could you change this to (whitespace and "strict mode"):

#!/usr/bin/env bash set -euo pipefail

Also, the ; at the end of each line isn't needed (it's only required when skipping the newline between multiple commands).

Btw a handy tool for bash scripts linting is:
https://github.com/koalaman/shellcheck

Oops, lost this in one my my resets.

edmorley · 2018-05-17T16:00:55Z

package.json

    "link:all": "lerna exec yarn link",
    "lint": "node packages/neutrino/bin/neutrino lint",
    "precommit": "lint-staged",
    "random": "node -e \"process.stdout.write(require('crypto').randomBytes(8).toString('hex'))\"",
-    "release": "yarn build && lerna publish --force-publish=*",
+    "release": "yarn build && NEUTRINO_RELEASE=true lerna publish --force-publish=*",


The pre-existing yarn build doesn't actually do anything as far as I can tell? Should we remove it (either now or at another time)?

edmorley · 2018-05-17T16:01:34Z

packages/create-project/test/cli_test.js

-import { packages } from '../commands/init/utils'
-
-if (process.env.NODE_ENV !== 'test') {
-  process.env.NODE_ENV = 'test';


I'm guessing this isn't needed since Neutrino sets NODE_ENV appropriately anyway?

We used it to drive stdio, but it was no longer used:

54c4a02#diff-00d28c6e44b222cb04e29211b7fffe8aL23

eliperelman · 2018-05-17T16:35:28Z

Alright, let's see what happens.

edmorley

The quoting change for yarn test fixed the issue for me btw :-)

edmorley · 2018-05-17T16:40:25Z

package.json

    "changelog": "auto-changelog --remote upstream --commit-limit false",
    "link:all": "lerna exec yarn link",
    "lint": "node packages/neutrino/bin/neutrino lint",
    "precommit": "lint-staged",
    "random": "node -e \"process.stdout.write(require('crypto').randomBytes(8).toString('hex'))\"",
-    "release": "yarn build && NEUTRINO_RELEASE=true lerna publish --force-publish=*",
+    "release": "SKIP_CHANGELOG=true lerna publish --force-publish=*",


The SKIP_CHANGELOG needs to be moved to test:create-project now

edmorley · 2018-05-17T17:30:22Z

package.json

-    "test:all": "nyc --reporter lcov ava --verbose --fail-fast packages/*/test",
+    "release": "lerna publish --force-publish=*",
+    "release:preview": "lerna publish --force-publish=* --skip-git --skip-npm",
+    "test": "ava --fail-fast packages/*/test \"!packages/create-project/test\"",


Could you document yarn test (and possibly yarn test:create-project, depending on how well it works locally) in development.md? :-)

edmorley · 2018-05-17T17:45:07Z

scripts/test-create-project-ci.sh

+export SKIP_CHANGELOG=true
+export YARN_AUTH_TOKEN="//localhost:4873/:_authToken=token"
+
+yarn verdaccio --config verdaccio.yml & sleep 10


One way to ensure verdaccio has started, but save spending the full 10 seconds if it was faster, would be to do:

yarn verdaccio --config verdaccio.yml & yarn config set registry "http://localhost:4873" # Wait until Verdaccio has started and is accepting connections. while ! nc -zw 1 localhost 4873; do sleep 1; done

(The wait line deliberately being after the yarn config set, since it gives a few more seconds for verdaccio to start, whilst also making use of the time, before sitting around idle)

edmorley · 2018-05-17T17:45:25Z

scripts/test-create-project-ci.sh

+
+yarn verdaccio --config verdaccio.yml & sleep 10
+yarn config set registry "http://localhost:4873"
+npm config set registry "http://localhost:4873"


The npm config line is no longer needed I think?

Since it's unused as of #877.

Use verdaccio to test create-project

8317e82

eliperelman requested a review from edmorley May 17, 2018 14:58

edmorley suggested changes May 17, 2018

View reviewed changes

Review revisions

58fdd07

Trying to automate publish

5a1eeac

edmorley reviewed May 17, 2018

View reviewed changes

eliperelman added 5 commits May 17, 2018 11:48

Move environment var to create-project script

f698567

Set yarn auth token in env

49dec2a

Set fake token against localhost

7675fc4

Publish major version in testing

1fcdebf

Remove extra deps, unused script, incorrect test file assertion

d541dd9

edmorley approved these changes May 17, 2018

View reviewed changes

Document verdaccio script and test commands

c500a29

eliperelman added the development label May 17, 2018

eliperelman merged commit ef553a9 into neutrinojs:master May 17, 2018

edmorley mentioned this pull request Jun 1, 2018

Remove nyc monorepo dependency #924

Merged

edmorley added a commit that referenced this pull request Jun 4, 2018

Remove nyc monorepo dependency (#924)

ce95903

Since it's unused as of #877.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use verdaccio to test create-project #877

Use verdaccio to test create-project #877

eliperelman commented May 17, 2018 •

edited

Loading

edmorley left a comment

edmorley May 17, 2018

edmorley May 17, 2018

eliperelman May 17, 2018

edmorley May 17, 2018

eliperelman May 17, 2018

eliperelman May 17, 2018

edmorley May 17, 2018

edmorley May 17, 2018

edmorley May 17, 2018

eliperelman May 17, 2018

edmorley May 17, 2018

edmorley May 17, 2018

eliperelman May 17, 2018

edmorley May 17, 2018

eliperelman May 17, 2018

edmorley May 17, 2018

eliperelman May 17, 2018

eliperelman commented May 17, 2018

edmorley left a comment

edmorley May 17, 2018

edmorley May 17, 2018

eliperelman May 17, 2018

edmorley May 17, 2018

edmorley May 17, 2018

Use verdaccio to test create-project #877

Use verdaccio to test create-project #877

Conversation

eliperelman commented May 17, 2018 • edited Loading

edmorley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eliperelman commented May 17, 2018

edmorley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eliperelman commented May 17, 2018 •

edited

Loading