Create dart.yml #82

domesticmouse · 2021-01-08T05:43:41Z

Created per @kevmoo's request in #81 (comment)

I started porting the travis test set to Github Actions and I hit something of a head scratcher. I realised that our Travis tests purport to run tests on windows... using a shell script runner. Which makes almost no sense. Unless the Travis windows machines have cygwin or similar installed.

Note, the windows tests appear to pass, even though some of the tests are specifically written for linux only (and test for the presence of a generated .so file ... as opposed to the .dll or .dylib that should be created on Windows and macOS respectively.

mit-mit · 2021-01-08T10:14:43Z

Thanks @domesticmouse! Can you please start by aligning with https://github.com/dart-lang/.github/blob/master/workflow-templates/test-package.yaml (there are some key differences, e.g. which setup task to use). @athomas fyi that Brett is looking at this repo.

domesticmouse · 2021-01-09T05:33:10Z

@mit-mit I'm unsure how I can align this script with another package when the intent was to port the current test suite from Travis to Github Actions.

@athomas if you want to own this and move it forward, please do so. At this point I don't understand how the current Travis test suite actually worked.

mit-mit · 2021-01-10T15:15:33Z

My comment was to align on steps that are actually similar, e.g. use uses: dart-lang/setup-dart@v0.1 not uses: cedx/setup-dart@v2

athomas · 2021-01-11T09:02:17Z

.github/workflows/dart.yml

+
+    steps:
+      - uses: actions/checkout@v2
+      - uses: cedx/setup-dart@v2


Prefer dart-lang/setup-dart@v0.1

Note that that will use channel instead of release-channel for the parameter.

athomas · 2021-01-11T09:02:56Z

.github/workflows/dart.yml

+        with:
+          release-channel: ${{ matrix.sdk }}
+
+      - name: Print Dart SDK version


Remove this step if you use dart-lang/setup-dart it will print the version it selects already in setup-dart.

athomas · 2021-01-11T09:05:19Z

.github/workflows/dart.yml

+      - name: Print Dart SDK version
+        run: dart --version
+
+      - name: "PKGS: command_line, extension_methods, ffi/hello_world, ffi/primitives, ffi/structs, ffi/system-command, native_app; TASKS: `dartanalyzer --fatal-infos --fatal-warnings .`"


You may want to move analysis and formatting to their own job (it doesn't need to run on all OS'es and can run before all other jobs). See https://github.com/dart-lang/.github/blob/master/workflow-templates/test-package.yaml#L15

athomas · 2021-01-11T09:06:55Z

.github/workflows/dart.yml

+      - name: Print Dart SDK version
+        run: dart --version
+
+      - name: "PKGS: command_line, extension_methods, ffi/hello_world, ffi/primitives, ffi/structs, ffi/system-command, native_app; TASKS: `dartanalyzer --fatal-infos --fatal-warnings .`"


Consider switching to dart analyze --fatal-infos, which should do the same as the legacy command you're currently using (--fatal-warnings is enabled by default in dart analyze).

athomas · 2021-01-11T09:07:43Z

.github/workflows/dart.yml

+          PKGS: "command_line extension_methods ffi/hello_world ffi/primitives ffi/structs ffi/system-command native_app"
+        run: ./tool/travis.sh dartanalyzer
+
+      - name: "PKGS: command_line, extension_methods, ffi/hello_world, ffi/primitives, ffi/structs, ffi/system-command, native_app; TASKS: `dartfmt -n --set-exit-if-changed .`"


Consider switching to dart format --output=none --set-exit-if-changed ., which should do the same as the legacy command you're currently using.

athomas · 2021-01-11T09:09:51Z

.github/workflows/dart.yml

+          PKGS: "command_line extension_methods ffi/hello_world ffi/primitives ffi/structs ffi/system-command native_app"
+        run: ./tool/travis.sh dartanalyzer
+
+      - name: "PKGS: command_line, extension_methods, ffi/hello_world, ffi/primitives, ffi/structs, ffi/system-command, native_app; TASKS: `dartfmt -n --set-exit-if-changed .`"


Consider running this even if analysis fails, because it's annoying to fix all your analysis errors only to discover that there is also a formatting error, see:
https://github.com/dart-lang/.github/blob/master/workflow-templates/test-package.yaml#L34

athomas · 2021-01-11T09:10:56Z

.github/workflows/dart.yml

+          PKGS: "command_line extension_methods ffi/hello_world ffi/primitives ffi/structs ffi/system-command native_app"
+        run: ./tool/travis.sh dartfmt
+
+      - name: "PKG: command_line, extension_methods, ffi/hello_world, ffi/primitives; TASKS: `pub run test`"


Consider switching to dart test.

athomas · 2021-01-11T09:14:26Z

I started porting the travis test set to Github Actions and I hit something of a head scratcher. I realised that our Travis tests purport to run tests on windows... using a shell script runner. Which makes almost no sense. Unless the Travis windows machines have cygwin or similar installed.

Not sure about travis, but GitHub Actions uses the bash shell coming with the Git installer for Windows. Most CI systems will install git that way and add it to the path, therefore having a bash executable even on Windows.

Use dart-lang/setup-dart@v0.1

domesticmouse · 2021-01-11T22:20:53Z

I started porting the travis test set to Github Actions and I hit something of a head scratcher. I realised that our Travis tests purport to run tests on windows... using a shell script runner. Which makes almost no sense. Unless the Travis windows machines have cygwin or similar installed.

Not sure about travis, but GitHub Actions uses the bash shell coming with the Git installer for Windows. Most CI systems will install git that way and add it to the path, therefore having a bash executable even on Windows.

That doesn't look like bash. And it should fail.

domesticmouse · 2021-01-12T01:08:41Z

Hey @johnpryan, I suspect I'm doing this backwards. Do you have some input on how we should do this?

@kevmoo does mono_repo support github actions?

johnpryan · 2021-01-12T01:36:10Z

Yeahmono_repo supports actions - I have an branch here, but it might be out of date.

domesticmouse · 2021-01-12T04:36:11Z

@johnpryan given you have a mono_repo based update that should handle this, I'm guessing I can close out this PR and let you land your change?

athomas · 2021-01-12T07:09:53Z

I started porting the travis test set to Github Actions and I hit something of a head scratcher. I realised that our Travis tests purport to run tests on windows... using a shell script runner. Which makes almost no sense. Unless the Travis windows machines have cygwin or similar installed.

Not sure about travis, but GitHub Actions uses the bash shell coming with the Git installer for Windows. Most CI systems will install git that way and add it to the path, therefore having a bash executable even on Windows.

That doesn't look like bash. And it should fail.

Yes, powershell is the default shell on windows for GitHub Actions, but bash is available: https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#using-a-specific-shell

That it works from powershell indicates the .sh extension is associated with some shell (probably sh). Though I haven't found any documentation that says it is. Requesting bash for the travis.sh steps is probably the best option.

johnpryan · 2021-01-19T23:45:49Z

Opening #83

Create dart.yml

d20b66b

domesticmouse requested a review from kevmoo January 8, 2021 05:44

domesticmouse mentioned this pull request Jan 8, 2021

Create dart.yml #81

Closed

athomas reviewed Jan 11, 2021

View reviewed changes

domesticmouse added 2 commits January 12, 2021 09:12

Update dart.yml

48744d7

Use dart-lang/setup-dart@v0.1

Update travis.sh

40c4d02

domesticmouse closed this Jan 19, 2021

kevmoo deleted the domesticmouse-patch-1 branch February 10, 2021 04:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create dart.yml #82

Create dart.yml #82

domesticmouse commented Jan 8, 2021 •

edited

mit-mit commented Jan 8, 2021

domesticmouse commented Jan 9, 2021

mit-mit commented Jan 10, 2021

athomas Jan 11, 2021

domesticmouse Jan 11, 2021

athomas Jan 11, 2021

domesticmouse Jan 11, 2021

athomas Jan 11, 2021

athomas Jan 11, 2021

athomas Jan 11, 2021

athomas Jan 11, 2021

athomas Jan 11, 2021

athomas commented Jan 11, 2021

domesticmouse commented Jan 11, 2021

domesticmouse commented Jan 12, 2021

johnpryan commented Jan 12, 2021

domesticmouse commented Jan 12, 2021

athomas commented Jan 12, 2021

johnpryan commented Jan 19, 2021

Create dart.yml #82

Create dart.yml #82

Conversation

domesticmouse commented Jan 8, 2021 • edited

mit-mit commented Jan 8, 2021

domesticmouse commented Jan 9, 2021

mit-mit commented Jan 10, 2021

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

athomas commented Jan 11, 2021

domesticmouse commented Jan 11, 2021

domesticmouse commented Jan 12, 2021

johnpryan commented Jan 12, 2021

domesticmouse commented Jan 12, 2021

athomas commented Jan 12, 2021

johnpryan commented Jan 19, 2021

domesticmouse commented Jan 8, 2021 •

edited