Skip to content

Adding github action tests#769

Merged
rwjblue merged 11 commits intoember-fastboot:masterfrom
kiwiupover:add-github-action-test
Jul 24, 2020
Merged

Adding github action tests#769
rwjblue merged 11 commits intoember-fastboot:masterfrom
kiwiupover:add-github-action-test

Conversation

@kiwiupover
Copy link
Member

@kiwiupover kiwiupover commented Jul 17, 2020

Adding github action tests

  • mocha tests against node 12, 10
  • ember tests
  • windows mocha test against node 12, 10
  • window ember tests

This is my github brach running these test. https://github.com/kiwiupover/ember-cli-fastboot/runs/883210806

The mocha window tests are having a hard time.
@rwjblue

- name: Install NPM version 4
run: |
npm config set spin false
npm install -g npm@4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😭

Comment on lines +64 to +65
- name: NPM Install
run: npm install
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it vaguely odd to use npm here. Can you swap to yarn?

Comment on lines +44 to +45
- name: NPM Install
run: npm install
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be running yarn not npm install.

See config in .travis.yml:

- yarn --ignore-engines

- name: Run Tests
run: npm run test:ember

test-windows-mocha-node:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since nothing is special/different about these mocha tests from the test-mocha job, we should consolidate them to a single job configuration above. I made an inline suggestion for that...

Comment on lines +25 to +28
test-mocha:
name: Mocha Tests
runs-on: ubuntu-latest

strategy:
matrix:
node-version: [12.x, 10.x]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid duplicate job definitions by using a matrix for OS also:

Suggested change
test-mocha:
name: Mocha Tests
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [12.x, 10.x]
test-mocha:
name: "Mocha - ${{ matrix.node-version }} - ${{ matrix.os }}"
runs-on: "${{ matrix.os }}-latest"
strategy:
matrix:
node-version: [12.x, 10.x]
os: [ubuntu, windows]

Comment on lines +88 to +89
- name: Remove Yarn Lock File
run: rm yarn.lock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't need to do this.

- name: Run Mocha Tests
run: npm run test:mocha

test-windows-ember:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be able to consolidate these in the job above with a matrix.os.

@kiwiupover kiwiupover force-pushed the add-github-action-test branch from 52cf10d to 4476b5b Compare July 23, 2020 06:47
Comment on lines +21 to +22
env:
CI: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I think this is automatically set now

@kiwiupover kiwiupover force-pushed the add-github-action-test branch from 77c2786 to 3441df7 Compare July 23, 2020 22:07
@kiwiupover
Copy link
Member Author

@rwjblue I have kept mocha tests split between Ubuntu and Windows because I can't get the Windows tests to pass on Github actions. I have combined the ember test into a matrix of os and node versions.

@kiwiupover kiwiupover force-pushed the add-github-action-test branch from fc4bb5e to 185b8d6 Compare July 24, 2020 15:26
@rwjblue rwjblue merged commit 1d5bb8f into ember-fastboot:master Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants