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.

Sign up for GitHub

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

Jump to bottom

Revamp GitHub workflows using Ember.js native implementation #144

Merged
merged 11 commits into from
Oct 13, 2021
Merged

Revamp GitHub workflows using Ember.js native implementation #144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8671a1b
ci: reset GH workflow to Ember's implementation
MrChocolatine Oct 12, 2021
fce0ae8
ci: manage Node.js version with an environment var
MrChocolatine Oct 12, 2021
1b51717
ci: use `yarn` for cached dependencies
MrChocolatine Oct 12, 2021
7b972d8
ci: run commands with `yarn` over `npm`
MrChocolatine Oct 12, 2021
5b643a2
ci: create a separate job `lint`
MrChocolatine Oct 12, 2021
6d4336f
ci: remove testing scenarios `beta` and `canary`
MrChocolatine Oct 12, 2021
312d539
refactor: space out things
MrChocolatine Oct 12, 2021
1e8ff88
ci(ember-try): update name of scenarios
MrChocolatine Oct 12, 2021
154a8cd
ci(ember-try): remove scenario `ember-default`
MrChocolatine Oct 12, 2021
8d18648
fix: fix syntax of npm-script `lint`
MrChocolatine Oct 12, 2021
cbe621c
ci: set jobs timeout to 20 minutes
MrChocolatine Oct 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
243 changes: 68 additions & 175 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,213 +1,106 @@
# This file was autogenerated by create-github-actions-setup-for-ember-addons.
#
# You can upgrade the GitHub Actions workflow to the latest blueprints used
# by Create GitHub Actions setup for Ember Addons by running it again:
#
# - `yarn create github-actions-setup-for-ember-addons` if using yarn and
# - `npm init github-actions-setup-for-ember-addons` if using NPM.
#
# See https://github.com/jelhan/create-github-actions-setup-for-ember-addon for
# details.
#
# The following lines contain the configuration used in the last run. Please do
# not change them. Doing so could break upgrade flow.
#
#$ browsers:
#$ - chrome
#$ emberTryScenarios:
#$ - scenario: lts-3.20
#$ - scenario: lts-3.24
#$ - scenario: release
#$ - scenario: default-with-jquery
#$ - scenario: classic
#$ nodeVersion: 12.x
#$ packageManager: yarn
#

name: CI

on:
push:
branches:
- main
- master
pull_request:
pull_request: {}

env:
NODE_VERSION: '12.x'
NODE_VERSION: '12'
Copy link
Member Author

Choose a reason for hiding this comment

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

The syntax 12 is enough if we just want to use the lastest major version.


jobs:
lint:
name: Lint
runs-on: ubuntu-latest
timeout-minutes: 20

steps:
- uses: actions/checkout@v2
with:
fetch-depth: 1

- uses: actions/setup-node@v2-beta
with:
node-version: '${{ env.NODE_VERSION }}'

- name: Get package manager's global cache path
id: global-cache-dir-path
run: echo "::set-output name=dir::$(yarn cache dir)"

- name: Cache package manager's global cache and node_modules
id: cache-dependencies
uses: actions/cache@v2
with:
path: |
${{ steps.global-cache-dir-path.outputs.dir }}
node_modules
key: ${{ runner.os }}-${{ matrix.node-version }}-${{
hashFiles('**/yarn.lock'
) }}
restore-keys: |
${{ runner.os }}-${{ matrix.node-version }}-

- name: Install Dependencies
run: yarn install --frozen-lockfile
if: |
steps.cache-dependencies.outputs.cache-hit != 'true'

- name: Lint
run: yarn lint
- uses: actions/checkout@v2
Copy link
Member Author

Choose a reason for hiding this comment

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

This Action already uses fetch-depth: 1 as its default:
https://github.com/actions/checkout/blob/v2.3.4/README.md?plain=1#L92-L94


- name: Install Node
uses: actions/setup-node@v2
with:
node-version: ${{ env.NODE_VERSION }}
cache: yarn
Copy link
Member Author

Choose a reason for hiding this comment

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

This allows us to let the Action handles the caching of dependencies:
https://github.com/actions/setup-node/blob/v2.4.1/docs/advanced-usage.md?plain=1#L45-L56


- name: Install Dependencies
run: yarn install --frozen-lockfile

- name: Lint
run: yarn run lint

test:
name: Tests
runs-on: ${{ matrix.os }}
needs: lint

strategy:
matrix:
os: [ubuntu-latest]
browser: [chrome]
runs-on: ubuntu-latest
timeout-minutes: 20

steps:
- uses: actions/checkout@v2
with:
fetch-depth: 1

- uses: actions/setup-node@v2-beta
with:
node-version: '${{ env.NODE_VERSION }}'

- name: Get package manager's global cache path
id: global-cache-dir-path
run: echo "::set-output name=dir::$(yarn cache dir)"

- name: Cache package manager's global cache and node_modules
id: cache-dependencies
uses: actions/cache@v2
with:
path: |
${{ steps.global-cache-dir-path.outputs.dir }}
node_modules
key: ${{ runner.os }}-${{ matrix.node-version }}-${{
hashFiles('**/yarn.lock'
) }}
restore-keys: |
${{ runner.os }}-${{ matrix.node-version }}-

- name: Install Dependencies
run: yarn install --frozen-lockfile
if: |
steps.cache-dependencies.outputs.cache-hit != 'true'

- name: Test
run: yarn test:ember --launch ${{ matrix.browser }}


floating-dependencies:
name: Floating Dependencies
runs-on: ${{ matrix.os }}
needs: lint
- uses: actions/checkout@v2

strategy:
matrix:
os: [ubuntu-latest]
browser: [chrome]
- name: Install Node
uses: actions/setup-node@v2
with:
node-version: ${{ env.NODE_VERSION }}
cache: yarn

steps:
- uses: actions/checkout@v2
with:
fetch-depth: 1
- name: Install Dependencies
run: yarn install --frozen-lockfile

- uses: actions/setup-node@v2-beta
with:
node-version: '${{ env.NODE_VERSION }}'
- name: Run Tests
run: yarn run test:ember
Copy link
Member Author

Choose a reason for hiding this comment

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

We used to run the following command: (source)

run: yarn test:ember --launch chrome

( ⬆️. because strategy.browser only has the value chrome)

I am not convinced --launch chrome is really needed, can someone confirm?

Copy link
Member

Choose a reason for hiding this comment

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

It seems useful when you have multiple browsers in testem.js launch_in_ci prop and you only want to run your test on one specific browser, as we only have Chrome in testem.js launch_in_ci prop, it seems ok to remove it

Copy link
Member Author

Choose a reason for hiding this comment

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

Noice, thanks.


- name: Get package manager's global cache path
id: global-cache-dir-path
run: echo "::set-output name=dir::$(yarn cache dir)"
floating:
name: Floating Dependencies
needs: lint
runs-on: ubuntu-latest
timeout-minutes: 20

- name: Cache package manager's global cache and node_modules
id: cache-dependencies
uses: actions/cache@v2
with:
path: |
${{ steps.global-cache-dir-path.outputs.dir }}
node_modules
key: ${{ runner.os }}-${{ matrix.node-version }}-floating-deps
restore-keys: |
${{ runner.os }}-${{ matrix.node-version }}-
steps:
- uses: actions/checkout@v2

- name: Install Dependencies
run: yarn install --no-lockfile --non-interactive
- uses: actions/setup-node@v2
with:
node-version: ${{ env.NODE_VERSION }}
cache: yarn

- name: Test
run: yarn test:ember --launch ${{ matrix.browser }}
- name: Install Dependencies
run: yarn install --no-lockfile --non-interactive

- name: Run Tests
run: yarn run test:ember
Copy link
Member Author

Choose a reason for hiding this comment

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

Same remark as https://github.com/peopledoc/ember-cli-embedded/pull/144/files#r727343324 .

Copy link
Member Author

Choose a reason for hiding this comment

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

Answer provided, see the related link ✅ .


try-scenarios:
name: Tests - ${{ matrix.ember-try-scenario }}
runs-on: ubuntu-latest
continue-on-error: true
name: ${{ matrix.try-scenario }}
Copy link
Member

Choose a reason for hiding this comment

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

As we are updating this file, wondering if we should add timeout-minutes option, to avoid having issues with some scenarios running for 360 minutes (default value) because there is an issue in tests and no output is generated, see https://github.com/peopledoc/ember-cli-embedded/runs/3443628551?check_suite_focus=true for example

Was happening mainly for Beta/Canary but could be a "security" for "ember-release" scenario 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, I actually thought about it yesterday and forgot to add it, thanks.

needs: test
runs-on: ubuntu-latest
timeout-minutes: 20

strategy:
fail-fast: true
matrix:
ember-try-scenario:
- lts-3.20
- lts-3.24
- release
- default-with-jquery
- classic
try-scenario:
- ember-lts-3.20
- ember-lts-3.24
- ember-release
- ember-classic
- ember-default-with-jquery
- embroider-safe
- embroider-optimized

steps:
- uses: actions/checkout@v2
with:
fetch-depth: 1

- uses: actions/setup-node@v2-beta
with:
node-version: '${{ env.NODE_VERSION }}'

- name: Get package manager's global cache path
id: global-cache-dir-path
run: echo "::set-output name=dir::$(yarn cache dir)"

- name: Cache package manager's global cache and node_modules
id: cache-dependencies
uses: actions/cache@v2
with:
path: |
${{ steps.global-cache-dir-path.outputs.dir }}
node_modules
key: ${{ runner.os }}-${{ matrix.node-version }}-${{
hashFiles('**/yarn.lock'
) }}
restore-keys: |
${{ runner.os }}-${{ matrix.node-version }}-

- name: Install Dependencies
run: yarn install --frozen-lockfile
if: |
steps.cache-dependencies.outputs.cache-hit != 'true'

- name: Test
env:
EMBER_TRY_SCENARIO: ${{ matrix.ember-try-scenario }}
run: node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO
- uses: actions/checkout@v2

- name: Install Node
uses: actions/setup-node@v2
with:
node-version: ${{ env.NODE_VERSION }}
cache: yarn

- name: Install Dependencies
run: yarn install --frozen-lockfile

- name: Run Tests
run: ./node_modules/.bin/ember try:one ${{ matrix.try-scenario }}
20 changes: 5 additions & 15 deletions config/ember-try.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,41 +9,31 @@ module.exports = async function () {
useYarn: true,
scenarios: [
{
name: 'lts-3.20',
name: 'ember-lts-3.20',
npm: {
devDependencies: {
'ember-source': '~3.20.5',
},
},
},
{
name: 'lts-3.24',
name: 'ember-lts-3.24',
npm: {
devDependencies: {
'ember-source': '~3.24.3',
},
},
},
{
name: 'release',
name: 'ember-release',
npm: {
devDependencies: {
'ember-source': await getChannelURL('release'),
},
},
},
// The default `.travis.yml` runs this scenario via `npm test`,
// not via `ember try`. It's still included here so that running
// `ember try:each` manually or from a customized CI config will run it
// along with all the other scenarios.
{
name: 'default',
npm: {
devDependencies: {},
},
},
{
name: 'default-with-jquery',
name: 'ember-default-with-jquery',
env: {
EMBER_OPTIONAL_FEATURES: JSON.stringify({
'jquery-integration': true,
Expand All @@ -56,7 +46,7 @@ module.exports = async function () {
},
},
{
name: 'classic',
name: 'ember-classic',
env: {
EMBER_OPTIONAL_FEATURES: JSON.stringify({
'application-template-wrapper': true,
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
},
"scripts": {
"build": "ember build --environment=production",
"lint": "npm-run-all --aggregate-output --continue-on-error --parallel 'lint:!(fix)'",
"lint": "npm-run-all --aggregate-output --continue-on-error --parallel \"lint:!(fix)\"",
"lint:fix": "npm-run-all --aggregate-output --continue-on-error --parallel lint:*:fix",
"lint:hbs": "ember-template-lint .",
"lint:hbs:fix": "ember-template-lint . --fix",
Expand Down