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

ci(win): setup GH actions for windows CI #4402

Merged
merged 7 commits into from
Sep 19, 2020

Conversation

outsideris
Copy link
Contributor

@outsideris outsideris commented Aug 9, 2020

Description of the Change

Replace AppVeyor with GitHub Actions for testing on Windows.
Because windows builds were failed on AppVeyor, I believe it is the platform issue, not ours, and AppVeyor introduced new Node versions sometimes, I set GitHub Actions for Windows CI.

You can see build results on my repository.

I tested actions/cache, but it spent more time than without cache since downloading cache files.

Benefits

We can have robust windows build results than before.

Possible Drawbacks

  • I set it up on Windows Server 2019. Do we need to set it up on Windows Server 2016 as well?
  • I didn't install Growl for Windows because all test already passed.
  • I added smoke tests and linting on Windows. I thought we need to do it even we run it on Travis. If you think it doesn't need on Windows, let me know.
  • GitHub provides 2,000 build minutes. Our Travis used 2,200 build mins last month and Travis build time is about 2 times than GitHub Actions Windows build. So, I think we can use GitHub Actions freely, but we will pay for Github Actions in the future.

Applicable issues

#4117
Close #4390

I want to hear feedback from @mochajs/core .

@coveralls
Copy link

coveralls commented Aug 9, 2020

Coverage Status

Coverage increased (+0.06%) to 93.885% when pulling 3d9b919 on outsideris:windows-actions into c1db2a6 on mochajs:master.

Copy link
Contributor

@craigtaub craigtaub left a comment

Choose a reason for hiding this comment

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

I added smoke tests and linting on Windows

Whats the value in adding these on Windows? Just thinking that it adds time and expense. Wondering if we have Windows users who get passing CI builds only to fail on release.

Any idea if there is a performance difference against AppVeyor?

That's great that we should get it free for now, what does AppVeyor cost currently (as that's a saving)?


## Build configuration
platform:
- x64
Copy link
Contributor

Choose a reason for hiding this comment

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

64-bit, so I'd assume windows server 2019 is enough by itself, but not sure.

## General configuration
version: '{build}'
skip_commits:
message: /\[ci\s+skip\]/
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the action skip too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check it.

@outsideris
Copy link
Contributor Author

Whats the value in adding these on Windows? Just thinking that it adds time and expense. Wondering if we have Windows users who get passing CI builds only to fail on release.

The purpose of smoke tests is to confirm that mocha will work when we release it. So, I thought it is better if we run smoke tests on Windows as well.

Linting is less useful than smoke tests. I'm not sure that failure to lint on windows while lint passed on Linux is possible. Just I want to check it for our windows contributors.

I can remove them from it if we agree it is useless.

Any idea if there is a performance difference against AppVeyor?

AppVeyor took about 22 mins and GitHub Actions took about 12 mins even though GitHub Actions have more tasks.
IMO, AppVeyor is free for us.

@craigtaub
Copy link
Contributor

AppVeyor took about 22 mins and GitHub Actions took about 12 mins even though GitHub Actions have more tasks.

🤩 🚀

So, I thought it is better if we run smoke tests on Windows as well.

I suppose with perf high and cost low its worth it. Linting I agree probably not necessary.

@outsideris outsideris force-pushed the windows-actions branch 2 times, most recently from 2bdfe59 to c7a7d5f Compare August 11, 2020 19:21
@boneskull
Copy link
Contributor

My only requirement is that it runs on Windows 10 and PowerShell. It should not be WSL or "Git Bash" or whatever.

If that's the case then I am 100000000% interested in merging this. 🥳

@outsideris
Copy link
Contributor Author

I'm trying to add a feature that skip CI with [ci skip]. Skipping CI based on branches is easy, but based on PRs is complex because GitHub doesn't provide the latest commit message on their context when triggered by PRs.

I will back after some test for that.

@outsideris
Copy link
Contributor Author

@boneskull I'm not familiar with Windows as development environment.
GitHub Actions only supports Windows Server 2016 and Windows Server 2019 and Windos Server 2019 looks similar with Windows 10.
And Powershell is the default shell for Windows in GitHub Actions.

powershell: This is the default shell used on Windows. The Desktop PowerShell.

@outsideris
Copy link
Contributor Author

Done.

Now, it will be skipped with [ci skip] commit message. You can check it in my repository for push and pull_request.

Retrive head commit message job always runs to get a head commit message when action triggered by pull request.

스크린샷 2020-08-17 오후 6 33 41

And I removed lint job from Windows CI.

@Munter
Copy link
Contributor

Munter commented Aug 17, 2020

I really like this addition. I just double checked out settings. Looks like we have 3000 minutes per month, apparently from a free open source team plan which must have been set up at some point.

We can also limit spending to $0 if we are afraid of running up a bill at some point. That would require someone to put down a credit card though, and I assume we don't have a cards associated with our opencollective profile, so it's probably an individual that needs to do this

@boneskull
Copy link
Contributor

I can put my feelers out and see if we can get more minutes if we start running into the limit.

@boneskull
Copy link
Contributor

sounds good @outsideris. travis has windows builds but they aren't powershell envs afaik. would be interested in moving everything to GHA eventually

restore-keys: |
${{ runner.os }}-growl-installer
- name: Download Growl Installer
if: steps.cache-growl.outputs.cache-hit != 'true'
Copy link
Contributor Author

@outsideris outsideris Sep 5, 2020

Choose a reason for hiding this comment

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

dowlnload Growl Installer only when the cache is not hit,

스크린샷 2020-09-05 오후 6 51 13

echo "Verifying Growl responding"
Start-Sleep -s 5
growlnotify test
echo "Staring test"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Start and verify growl before running the test because GH actions don't keep growl process previous steps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

스크린샷 2020-09-05 오후 7 03 48

Sometimes, there are DllNotFoundException, not always. But we can verify Growl responding regardless the exception.

@outsideris
Copy link
Contributor Author

I added Growl installer and cache it which mentioned #4391

@outsideris
Copy link
Contributor Author

It is ready to merge.

@outsideris
Copy link
Contributor Author

I will merge this in a few days.

@Munter
Copy link
Contributor

Munter commented Sep 20, 2020

Great work on this! Do good to have both speed increases and better access for everyone to see the results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cache growl on appveyor
5 participants