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

[Hold] Add 'rwjs/redwood' Repo CI via GH Actions: Build, Lint, and Jest #110

Closed
wants to merge 22 commits into from

Conversation

thedavidprice
Copy link
Contributor

@thedavidprice thedavidprice commented Feb 11, 2020

Closes #105

@thedavidprice
Copy link
Contributor Author

Whelp... it's a start I guess. TBD

@thedavidprice thedavidprice self-assigned this Feb 11, 2020
@thedavidprice
Copy link
Contributor Author

thedavidprice commented Feb 13, 2020

In future, will need to implement caching to improve performance:

Ran into issues with GH Action caching not yet possible with yarn v1 workspaces. Conceivably could config the Action to run individually on each package (check cache, install, build, test), but that seems like a pain for now.

Related:

@thedavidprice
Copy link
Contributor Author

thedavidprice commented Feb 13, 2020

Aside from the fact the few test we have are failing 😉, this is now working for tests on Node 10 latest (Dubnium) and Node 12 latest (Erbium). For both environments, the action runs yarn build and yarn test.

It’s slow without caching. And without test coverage might not be all that useful yet. But ready to go when we are.

If we do decide to merge, we should:

  • fix current failing tests
  • take a stab at any mission critical tests (maybe)

@thedavidprice thedavidprice changed the title [WIP] Adding Jest Tests CI with GH Workflow+Actions Adding Jest Tests CI with GH Actions Feb 13, 2020
@thedavidprice
Copy link
Contributor Author

Notes for possible improvements:

  • employ something like lerna changed to build against modified packages only (need to choose what our reference will be… last npm release is more than needed but might be good easy first step. ideally just new commits.)
  • separate into two Actions: 1) build and 2) test

Another Action workflow we might get easily is opening a PR for all our other projects needing to update package versions.

@thedavidprice
Copy link
Contributor Author

@peterp Take a look at the build errors I’m now getting running against latest master. Looks like failing on cli/… possibly something to do with permissions on the GH Actions container? (Actions are painful to debug… so looking for some intuition before I dive in.)

Screen Shot 2020-02-24 at 1 06 13 PM

@thedavidprice
Copy link
Contributor Author

Fixed an error caused by directory name mismatch between uppercase cli/src/commands/Generators/ and actual ..generators/. Also changed ..dbGenerators/ to db_generators. Then adjusted import statements accordingly.

Builds are now passing.

Added Linting.

Linting and Jest are running but failing.

@thedavidprice
Copy link
Contributor Author

yarn lint is generating the following error repeated about 10x:

[eslint-import-resolver-babel-module] TypeError: Cannot read property 'plugins' of null
    at getPlugins (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-import-resolver-babel-module/lib/index.js:23:19)
    at getPluginOptions (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-import-resolver-babel-module/lib/index.js:35:21)
    at Object.exports.resolve (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-import-resolver-babel-module/lib/index.js:94:27)
    at v2 (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-module-utils/resolve.js:117:23)
    at withResolver (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-module-utils/resolve.js:122:16)
    at fullResolve (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-module-utils/resolve.js:139:22)
    at relative (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-module-utils/resolve.js:84:10)
    at resolve (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-module-utils/resolve.js:220:12)
    at resolveImportType (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-plugin-import/lib/core/importType.js:148:65)
    at computeRank (/Users/price/Repos/redwoodjs-redwood/node_modules/eslint-plugin-import/lib/rules/order.js:317:44)
See: https://github.com/tleunen/eslint-import-resolver-babel-module/pull/34

I took a look at tleunen/eslint-import-resolver-babel-module#34 which basically says the warning/error is just a wrapper around Babel do indicate something went wrong with the Babel config. In our case the error TypeError: Cannot read property 'plugins' of null is coming from Babel. I was unable to diagnose so far.

@peterp
Copy link
Contributor

peterp commented Mar 3, 2020

I personally don't like using snake_case file naming conventions in javascript projects. The reason for this is that we don't generally name our variables or functions in snake case and I try to name the file the same as the main exported function.

The way that I change cases with git is to use two commits.

git mv Generate xGenerate
git commit -am 'Renaming Generate -> generate part 1.'
git mv xGenerate generate
git commit -am 'Renaming Generate -> generate part 2.'

I think we should open this for discussion and figure out what standard we would like to apply to the project as a whole.

@thedavidprice thedavidprice changed the title Adding Jest Tests CI with GH Actions Add 'rwjs/redwood' Repo CI via GH Actions: Build, Lint, and Jest Mar 4, 2020
@thedavidprice thedavidprice changed the title Add 'rwjs/redwood' Repo CI via GH Actions: Build, Lint, and Jest [Hold] Add 'rwjs/redwood' Repo CI via GH Actions: Build, Lint, and Jest Mar 4, 2020
@thedavidprice
Copy link
Contributor Author

closing this in favor of #257

Will be creating separate issues for:

  1. next steps to enhance CI
  2. linter warnings related to bable config error

@thedavidprice thedavidprice deleted the dsp-gh-actions-CI branch March 15, 2020 00:13
@thedavidprice thedavidprice restored the dsp-gh-actions-CI branch April 1, 2020 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

setting up CI/CD runs for redwood Repo
2 participants