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

Add windows tests to CircleCI #361

Merged
merged 1 commit into from
Apr 22, 2020
Merged

Add windows tests to CircleCI #361

merged 1 commit into from
Apr 22, 2020

Conversation

mdeggies
Copy link
Member

@mdeggies mdeggies commented Apr 17, 2020

This PR adds another job to run the go tests on a Windows executor, which will let us get rid of the appveyor dependency. To run tests with different versions of go, add the version(s) to the array's on lines 170 and 175.

The linux and win tests run at the same time, but the Windows job does take a bit longer to finish due to the nature of the executor + some required setup steps. The workflow takes around ~1:23 to complete. The longest step (w/out caching) is installing go, which we have to do because the Windows executor only has go 1.12 installed. This job has gomod + coverage persistence + golang caching enabled, so it's a bit quicker.

Besides removing the appveyor file, I also removed the travis config file in this PR, as the circle config seems to cover all tests for branch pushes/pr's/merges.

Screen Shot 2020-04-16 at 10 37 07 PM

Let me know if you see anything wonky, and thanks for your review!

@mdeggies mdeggies requested a review from alisdair April 17, 2020 05:37
Copy link
Member

@alisdair alisdair left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! I found one thing I think we should fix before merge, left a comment inline.

We'll also need to port this to the more active hcl2 branch, but I think that might just be a cherry-pick one the commits are squashed.

- run: go mod verify
- run: make fmt
- run:
name: verify no code was generated
Copy link
Member

Choose a reason for hiding this comment

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

This is where I wrote a comment and GitHub seems to have deleted it for some reason, which is pretty rude. An attempt to reconstruct:

Looking at the test output, we're not running this command in the Linux tests. And in the Windows tests, we don't run go mod verify or make fmt before it, so it's always going to pass. It'd be great to have this check present and working on at least one of the platforms.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh good catch! I forgot to add that job to the workflows. I'll have the check job run before the tests on windows & linux.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed & squashed my commits- let me know how it looks! Can cherrypick once this is merged or open another PR (unless that's overkill).

Screen Shot 2020-04-21 at 6 13 19 PM

Copy link
Member

@alisdair alisdair left a comment

Choose a reason for hiding this comment

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

Great, thank you! 🎉 Cherry-picking onto the hcl2 branch should be fine I think, no need to PR.

@mdeggies mdeggies merged commit 569ae81 into master Apr 22, 2020
@mitchellh mitchellh deleted the add-win-tests branch October 22, 2020 23:42
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.

None yet

2 participants