FEA-318: Use github actions instead of travis for CI #189

kimlarson-wk · 2022-06-14T18:10:03Z

FEA-318

Motivation

The w_module repo is currently using travis for CI. We don't use that anymore.

Changes

Used github actions instead of travis. Added a skynet.yaml.

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

Tests were updated and provide good coverage of the changeset and other affected code
Manual testing was performed if needed
- Steps from PR author:
- Anything falling under manual testing criteria outlined in CONTRIBUTING.md

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

A Frontend Architecture member has reviewed these changes
There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
For release PRs - Version metadata in Rosie comment is correct

FEA-318

aviary-wf · 2022-06-14T18:12:31Z

Security Insights

No security relevant content was detected by automated scans.

Action Items

Review PR for security impact; comment "security review required" if needed or unsure
Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

FEA-318

corwinsheahan-wf

just a couple of small things

corwinsheahan-wf · 2022-06-14T18:57:44Z

.travis.yml

-        - pub run dart_dev format --check
-        - pub run dart_dev analyze
-        - pub run dart_dev test
-        - pub run dart_dev test --release


Since this workflow used dart_dev, we'll probably want to do that in the github workflow as well.

I'm confused on why no other workflow does it 🤔

Likely because most of them don't actually depend on dart_dev. Looks like at least w_transport uses it.

But in general, running dart run dart_dev test is just any other CLI command. There's no reason it can't or shouldn't be used in a github workflow.

corwinsheahan-wf · 2022-06-14T18:58:26Z

test/lifecycle_module_test.dart

@@ -632,17 +632,15 @@ void runTests(bool runSpanTests) {
        expect(
            Logger.root.onRecord,
            emits(
-                logRecord(level: Level.WARNING, message: contains('loading'))));
+                logRecord(level: Level.CONFIG, message: contains('loading'))));


What caused this change?

Ah, looks like we had a PR go through which updated a log level: https://github.com/Workiva/w_module/pull/181/files

That was able to sneak through because tests weren't actually being ran.

corwinsheahan-wf · 2022-06-14T19:05:21Z

skynet.yaml

+name: w_module-unit-tests
+description: Unit tests
+contact: 'Frontend Frameworks Architecture / #support-frontend-architecture'
+image: drydock.workiva.net/workiva/dart_unit_test_image:1
+size: large
+timeout: 600
+
+artifacts: /testing/w_module/test-reports
+test-reports: /testing/w_module/test-reports
+
+scripts:
+  - cd /testing/w_module/
+  - ./run_tests.sh


Since we already run tests in github workflows, perhaps just no-op here.

What's the benefit of having them run via workflows rather than skynet?

I believe it's so that open source contributors can see the results of a CI run (skynet requires privileged access).

FEA-318

.github/workflows/dart_ci.yml

evanweible-wf

Was there a reason to move the tests from test/ to test/unit/? In many of our smaller packages, we only have unit tests, so putting them directly in test/ is sufficient/simpler.

FEA-318

corwinsheahan-wf

I think we should avoid moving things under test/ to test/unit.

corwinsheahan-wf · 2022-06-15T16:54:26Z

skynet.yaml

+  on-pull-request: true
+
+scripts:
+  - echo "No tests currently set"


Suggested change

- echo "No tests currently set"

- echo "Tests run in a Github Workflow"

corwinsheahan-wf · 2022-06-15T16:57:16Z

test/lifecycle_module_test.dart

@@ -632,17 +632,15 @@ void runTests(bool runSpanTests) {
        expect(
            Logger.root.onRecord,
            emits(
-                logRecord(level: Level.WARNING, message: contains('loading'))));
+                logRecord(level: Level.CONFIG, message: contains('loading'))));


Ah, looks like we had a PR go through which updated a log level: https://github.com/Workiva/w_module/pull/181/files

That was able to sneak through because tests weren't actually being ran.

.github/workflows/dart_ci.yml

corwinsheahan-wf · 2022-06-15T17:59:40Z

.github/workflows/dart_ci.yml

+        run: dart pub run dart_dev analyze
+        if: always() && steps.install.outcome == 'success'
+
+      - name: Run tests with ddc
+        run: dart pub run dart_dev test
+        if: always() && steps.install.outcome == 'success'
+
+      - name: Run tests with dart2js
+        run: dart pub run dart_dev test --release
+        if: always() && steps.install.outcome == 'success'
+
+  format:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: dart-lang/[email protected]
+        with:
+          sdk: 2.13.4
+
+      - id: install
+        name: Install dependencies
+        run: dart pub get
+
+      - name: Verify formatting
+        run: dart pub run dart_dev format --check
+        if: always() && steps.install.outcome == 'success'


#nit the dart cli supports run directly, so I believe we can omit the pub in these commands

Suggested change

run: dart pub run dart_dev analyze

if: always() && steps.install.outcome == 'success'

- name: Run tests with ddc

run: dart pub run dart_dev test

if: always() && steps.install.outcome == 'success'

- name: Run tests with dart2js

run: dart pub run dart_dev test --release

if: always() && steps.install.outcome == 'success'

format:

runs-on: ubuntu-latest

steps:

- uses: actions/checkout@v2

- uses: dart-lang/[email protected]

with:

sdk: 2.13.4

- id: install

name: Install dependencies

run: dart pub get

- name: Verify formatting

run: dart pub run dart_dev format --check

if: always() && steps.install.outcome == 'success'

run: dart run dart_dev analyze

if: always() && steps.install.outcome == 'success'

- name: Run tests with ddc

run: dart run dart_dev test

if: always() && steps.install.outcome == 'success'

- name: Run tests with dart2js

run: dart run dart_dev test --release

if: always() && steps.install.outcome == 'success'

format:

runs-on: ubuntu-latest

steps:

- uses: actions/checkout@v2

- uses: dart-lang/[email protected]

with:

sdk: 2.13.4

- id: install

name: Install dependencies

run: dart pub get

- name: Verify formatting

run: dart run dart_dev format --check

if: always() && steps.install.outcome == 'success'

FEA-318

.github/workflows/dart_ci.yml

FEA-318

Comments addressed

evanweible-wf · 2022-06-15T22:07:29Z

QA +1

CI passes

evanweible-wf · 2022-06-15T22:07:35Z

@Workiva/release-management-p

rmconsole-wf

+1 from RM

Use github actions instead of travis for CI

ecf006f

FEA-318

rmconsole-wf added the Merge Requirements Unmet label Jun 14, 2022

rmconsole-wf added the Skynet 2.0 Failed label Jun 14, 2022

kimlarson-wk added 4 commits June 14, 2022 13:14

No need to change directories before running commands

1ea8937

FEA-318

Add dependency_validator

8d85d02

FEA-318

Fix deps

853e844

FEA-318

Logs are being sent as config, not as warning.

05ec5a0

FEA-318

kimlarson-wk marked this pull request as ready for review June 14, 2022 18:42

kimlarson-wk requested a review from a team June 14, 2022 18:42

kimlarson-wk changed the title ~~Use github actions instead of travis for CI~~ FEA-318: Use github actions instead of travis for CI Jun 14, 2022

Add script for running tests

2bba913

FEA-318

corwinsheahan-wf requested changes Jun 14, 2022

View reviewed changes

No need to run unit tests on both skynet and workflows

011e5aa

FEA-318

rmconsole-wf removed the Skynet 2.0 Failed label Jun 15, 2022

Checking if the older dart version is what caused dart pub get to fail

8b418b0

FEA-318

kimlarson-wk commented Jun 15, 2022

View reviewed changes

.github/workflows/dart_ci.yml Show resolved Hide resolved

kimlarson-wk requested a review from corwinsheahan-wf June 15, 2022 15:22

evanweible-wf reviewed Jun 15, 2022

View reviewed changes

Readd dart2js tests

8f92c16

FEA-318

corwinsheahan-wf previously requested changes Jun 15, 2022

View reviewed changes

kimlarson-wk requested review from evanweible-wf and corwinsheahan-wf June 15, 2022 17:54

corwinsheahan-wf reviewed Jun 15, 2022

View reviewed changes

kimlarson-wk force-pushed the FEA-318_UseGithubActions branch from ac928f4 to ec4d09f Compare June 15, 2022 18:23

Use dart_dev

980595b

FEA-318

kimlarson-wk force-pushed the FEA-318_UseGithubActions branch from 195bfc7 to 980595b Compare June 15, 2022 18:50

kimlarson-wk added 2 commits June 15, 2022 13:53

dart pub run -> dart run

be6e639

FEA-318

Add build_vm_compilers dep

a85ba29

FEA-318

todbachman-wf reviewed Jun 15, 2022

View reviewed changes

.github/workflows/dart_ci.yml Show resolved Hide resolved

kimlarson-wk added 3 commits June 15, 2022 15:50

Use build_runner for now

0fcd32d

FEA-318

Bump deps to use dart_dev

5cd60fa

FEA-318

Bump dependencies further. Readd removed ones.

7ac515b

FEA-318

todbachman-wf approved these changes Jun 15, 2022

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Jun 15, 2022

evanweible-wf approved these changes Jun 15, 2022

View reviewed changes

rmconsole-wf added the RM Ready label Jun 15, 2022

rmconsole-wf approved these changes Jun 15, 2022

View reviewed changes

rmconsole2-wf merged commit 892e6b1 into master Jun 15, 2022

rmconsole2-wf deleted the FEA-318_UseGithubActions branch June 15, 2022 22:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEA-318: Use github actions instead of travis for CI #189

FEA-318: Use github actions instead of travis for CI #189

kimlarson-wk commented Jun 14, 2022 •

edited

Loading

aviary-wf commented Jun 14, 2022

corwinsheahan-wf left a comment

corwinsheahan-wf Jun 14, 2022

kimlarson-wk Jun 14, 2022

corwinsheahan-wf Jun 15, 2022

corwinsheahan-wf Jun 15, 2022

corwinsheahan-wf Jun 14, 2022

corwinsheahan-wf Jun 15, 2022

corwinsheahan-wf Jun 14, 2022

kimlarson-wk Jun 14, 2022

corwinsheahan-wf Jun 14, 2022

kimlarson-wk Jun 14, 2022

evanweible-wf left a comment

corwinsheahan-wf left a comment

corwinsheahan-wf Jun 15, 2022 •

edited

Loading

corwinsheahan-wf Jun 15, 2022

corwinsheahan-wf Jun 15, 2022

evanweible-wf commented Jun 15, 2022

evanweible-wf commented Jun 15, 2022

rmconsole-wf left a comment

	- echo "No tests currently set"
	- echo "Tests run in a Github Workflow"

FEA-318: Use github actions instead of travis for CI #189

FEA-318: Use github actions instead of travis for CI #189

Conversation

kimlarson-wk commented Jun 14, 2022 • edited Loading

Motivation

Changes

Release Notes

Review

QA Checklist

Merge Checklist

aviary-wf commented Jun 14, 2022

Security Insights

Action Items

corwinsheahan-wf 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

evanweible-wf left a comment

Choose a reason for hiding this comment

corwinsheahan-wf left a comment

Choose a reason for hiding this comment

corwinsheahan-wf Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanweible-wf commented Jun 15, 2022

evanweible-wf commented Jun 15, 2022

rmconsole-wf left a comment

Choose a reason for hiding this comment

kimlarson-wk commented Jun 14, 2022 •

edited

Loading

corwinsheahan-wf Jun 15, 2022 •

edited

Loading