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

Fix windows tests that are failing CI on main #5916

Merged
merged 4 commits into from
Dec 6, 2023
Merged

Conversation

erikbaranowski
Copy link
Contributor

@erikbaranowski erikbaranowski commented Dec 5, 2023

PR Description

Fixes broken windows tests possibly coming from: #5590 & #5753

We will want to follow up on the second one which is disabled for windows in this PR to get CI working again.

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@@ -1,3 +1,5 @@
//go:build !windows
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem with the integration tests is that they cannot run in Drone. They only run in github Actions (because it uses docker compose which is not possible in Drone). I was planning to use test-containers (see #5861) but now that we are moving away from Drone I will probably keep them in Github Actions. The reason why the tests are running is because the Windows pipeline is calling "go test" at the root of the pipeline and that triggers the integration tests (https://github.com/grafana/agent/blob/main/.drone/drone.yml#L184). They dont run when you run make test because I made a special exclusion (https://github.com/grafana/agent/blob/main/Makefile#L176). I wanted to do the same thing to fix the windows pipeline but I modified directly the yml file not knowing that it was a generated file (I just realised now) https://github.com/grafana/agent/pull/5643/files. Now Im not sure whether this fix actually work or if that's the right approach (@tpaschalis suggested also to use build tags but you need to make sure that the nothing runs in the integration tests folder)

Copy link
Contributor

@wildum wildum left a comment

Choose a reason for hiding this comment

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

Approval for the integration tests part

@erikbaranowski erikbaranowski merged commit 400c2be into main Dec 6, 2023
10 checks passed
@erikbaranowski erikbaranowski deleted the fix-windows-tests branch December 6, 2023 15:46
BarunKGP pushed a commit to BarunKGP/grafana-agent that referenced this pull request Feb 20, 2024
* update loki client test for windows differences

* disable integration-tests that don't work in windows when running in windows
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 21, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants