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

feat(ci): use GitHub Actions for CI/CD #8775

Closed
wants to merge 1 commit into from
Closed

Conversation

paul-marechal
Copy link
Member

@paul-marechal paul-marechal commented Nov 23, 2020

Replace .travis.yml with GitHub Actions.

Splitting scripts was needed to correctly run in CI:

  • yarn now only install deps and builds example apps.
  • yarn lint lints our TypeScript sources.

THEIA_SKIP_BUILD=1 can be enabled to only install deps when doing yarn.

Notes:

I skipped the flakiest tests, but it seems like some are still causing trouble. Feel free to restart the jobs.

@paul-marechal paul-marechal marked this pull request as ready for review November 23, 2020 20:47
@paul-marechal paul-marechal force-pushed the mp/gh-actions branch 10 times, most recently from a25807f to eed9638 Compare November 24, 2020 05:08
@kittaakos kittaakos self-requested a review November 24, 2020 08:45
@vince-fugnitto vince-fugnitto added the ci issues related to CI / tests label Nov 24, 2020
@paul-marechal paul-marechal force-pushed the mp/gh-actions branch 15 times, most recently from a80a029 to 33b181d Compare November 24, 2020 20:23
@paul-marechal paul-marechal force-pushed the mp/gh-actions branch 16 times, most recently from f514a71 to bf06d64 Compare November 25, 2020 07:01
Replace `.travis.yml` with GitHub Actions.

Splitting scripts was needed to correctly run in CI:
- `yarn` now only install deps and builds example apps.
- `yarn lint` lints our TypeScript sources.

`THEIA_SKIP_BUILD=1` can be enabled to only install deps when doing `yarn`.
@paul-marechal paul-marechal changed the title wip: Use GitHub Actions feat(ci): use GitHub Actions for CI/CD Nov 25, 2020
@paul-marechal
Copy link
Member Author

@kittaakos just a heads up that I think this PR is ready for review.

What's done:

  • Install, lint, build, and test.
  • Publishing next? Wasn't able to test, and I think we are missing a secret (NPM_TOKEN).

What's missing:

  • Caching of node_modules.
  • Publishing the gh_pages.

I think the missing parts can be added later, but feel free to push on this branch to add things you think should be there.

@paul-marechal
Copy link
Member Author

cc @vince-fugnitto

@kittaakos
Copy link
Contributor

Thank you for preparing it, @marechal-p. There are a few things I do not really like: such as unnecessary configs, changed npm scripts, hacks, and the publish build does not wait. Here is my proposal. There is one thing I do not understand, the macOS build fails for some reason.

$ yarn build && run lint && run build "@theia/example-*" --stream --parallel
$ tsc -b configs/root-compilation.tsconfig.json
ERR! lerna Invalid command!
ERR! lerna Pass --help to see all available commands and options.

Here is the build without the macOS image, and here is with macOS.

I also updated the issue for this PR: we do the codec thingy after the tsc build. Is that correct? I guess you are the one who has the most experience with the code logic.

@kittaakos
Copy link
Contributor

kittaakos commented Nov 25, 2020

There is one thing I do not understand, the macOS build fails for some reason.

There is something odd with the theia-run.js module when manipulating the proceee.argv:

Here are the outputs from Linux and macOS:

macOS:

process.argv: ["/Users/runner/hostedtoolcache/node/12.19.0/x64/bin/node","/Users/runner/work/theia/theia/node_modules/.bin/run","lint"]
args: ["/Users/runner/work/theia/theia/node_modules/.bin/run","lint"]
scopedArgs: ["/Users/runner/work/theia/theia/node_modules/.bin/run","lint"]
modified process.argv: ["/Users/runner/hostedtoolcache/node/12.19.0/x64/bin/node","run","/Users/runner/work/theia/theia/node_modules/.bin/run","--scope","lint"]

Linux:

process.argv: ["/opt/hostedtoolcache/node/12.19.0/x64/bin/node","/home/runner/work/theia/theia/node_modules/.bin/run","lint"]
args: ["lint"]
scopedArgs: ["lint"]
modified process.argv: ["/opt/hostedtoolcache/node/12.19.0/x64/bin/node","/home/runner/work/theia/theia/node_modules/.bin/run","run","lint"]

Windows:

process.argv: ["C:\\hostedtoolcache\\windows\\node\\12.19.0\\x64\\node.exe","D:\\a\\theia\\theia\\dev-packages\\ext-scripts\\theia-run.js","lint"]
args: ["C:\\hostedtoolcache\\windows\\node\\12.19.0\\x64\\node.exe","D:\\a\\theia\\theia\\dev-packages\\ext-scripts\\theia-run.js","lint"]
scopedArgs: ["C:\\hostedtoolcache\\windows\\node\\12.19.0\\x64\\node.exe","D:\\a\\theia\\theia\\dev-packages\\ext-scripts\\theia-run.js","lint"]
modified process.argv: ["run","C:\\hostedtoolcache\\windows\\node\\12.19.0\\x64\\node.exe","--scope","D:\\a\\theia\\theia\\dev-packages\\ext-scripts\\theia-run.js","lint"]

@kittaakos
Copy link
Contributor

but feel free to push on this branch to add things you think should be there.

Here is an alternative solution: #8780

@paul-marechal
Copy link
Member Author

paul-marechal commented Nov 25, 2020

There is something odd with the theia-run.js module when manipulating the proceee.argv:

Yes and since I did not have access to a mac I avoided this issue by not using theia-run.js. I see that you have a fix, nice.

we do the codec thingy after the tsc build. Is that correct?

It happens on postinstall for the @theia/electron package. From the commit history, we had to explicitly call the ffmepg test scripts because the @theia/electron package was being cached, so postinstall did not run somehow? At any rate, the tests should happen when we do yarn on the monorepo.

@paul-marechal
Copy link
Member Author

Closing in favor of #8780

@paul-marechal paul-marechal deleted the mp/gh-actions branch December 8, 2020 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci issues related to CI / tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants