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

Windows CI Integration #702

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Windows CI Integration #702

wants to merge 24 commits into from

Conversation

tonerdo
Copy link
Contributor

@tonerdo tonerdo commented May 13, 2019

This PR sets up a Windows CI for Terragrunt using Azure Pipelines (replacing AppVeyor).

We're able to split into unit and integration test jobs
Screenshot 2019-05-13 at 10 06 59 PM

We can also publish XML test results generated by the Terratest log parser
Screenshot 2019-05-13 at 10 10 16 PM

Unlike AppVeyor (and like CircleCI) we can have organizations
Screenshot 2019-05-13 at 10 13 47 PM

Some tests are failing of course, but the point of this PR is to get the Windows CI infrastructure setup

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

This is great, thanks Toni! 😁

Just a couple questions.


- pwsh: |
if ("$(TESTKIND)" -eq "unit") {
go test -v -parallel 1 $(go list ./... | where-object { $_ -notlike "*/test" }) | Tee-Object -FilePath 'logs/unit.log'
Copy link
Member

Choose a reason for hiding this comment

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

I think we normally use run-go-test for this, which sets other options, such as -timeout. Can you make sure we re-use those here?

Also, perhaps a dumb question, but is there no way we can reuse any of those Bash scripts here? I'm curious if we can reduce duplication at all between Windows and *nix.

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 can run it on "Bash for Windows" (https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=azure-devops&tabs=schema#bash). The problem with this approach is, the tests will still be running in the linux subsystem and not on Windows directly. That negates the whole point of setting up a Windows CI

Copy link
Member

Choose a reason for hiding this comment

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

Ah, OK, fair enough

Copy link
Contributor

@yorinasub17 yorinasub17 May 14, 2019

Choose a reason for hiding this comment

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

Should we use python for our CI scripts once we have more windows CI tasks, to avoid the code duplication?

Copy link
Member

Choose a reason for hiding this comment

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

Yea, probably. I mean, that still requires Python to be installed, and there will still be other OS differences to paper over (e.g., path lengths), but that should help.

Copy link
Contributor

Choose a reason for hiding this comment

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

At least it isn't "share no code" 😄

Copy link

Choose a reason for hiding this comment

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

Why not Golang? Installing Python is going to be a mess, someone's bound to install something and complain...

}

if ("$(TESTKIND)" -eq "integration") {
go test -v $(go list ./... | where-object { $_ -like "*/test" }) | Tee-Object -FilePath 'logs/integration.log'
Copy link
Member

Choose a reason for hiding this comment

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

Same here

displayName: 'Run `dep ensure`'

- pwsh: |
if ("$(TESTKIND)" -eq "unit") {
Copy link
Member

Choose a reason for hiding this comment

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

What language is this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Powershell

}
workingDirectory: '$(modulePath)'
displayName: 'Run tests'
continueOnError: true
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need this here so that the task that publishes the xml test results will run. The entire build pipeline is still marked as failed so it doesn't change much

Copy link
Member

Choose a reason for hiding this comment

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

Roger

$(system.defaultWorkingDirectory)\terratest_log_parser.exe --testlog logs/integration.log --outputdir ./logs/integration/
}
workingDirectory: '$(modulePath)'
displayName: 'Generate Test Report'
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

@tonerdo
Copy link
Contributor Author

tonerdo commented May 14, 2019

@brikis98 I invited you to the Gruntwork Azure DevOps org. Let me know if you have any trouble

@tonerdo
Copy link
Contributor Author

tonerdo commented Jun 3, 2019

Just checking if this is something we're still interested in. So I can properly handover ownership of the Azure DevOps Gruntwork organization

@brikis98
Copy link
Member

brikis98 commented Jun 5, 2019

Yes, thanks for checking @tonerdo! I've just been on vacation and haven't had time to dig into this stuff yet. And now #466 is a high priority, so I need to wrap that up, and will then come back to this.

@azilber
Copy link

azilber commented Feb 24, 2020

Any update on this? Building out a pipeline and wondering; when this is going to be merged?

@zackproser zackproser self-assigned this Jan 18, 2022
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.

5 participants