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

Prevent github workflow from failing if windows integration tests fail #6704

Conversation

JoshVanL
Copy link
Contributor

@JoshVanL JoshVanL marked this pull request as ready for review July 20, 2023 17:36
@JoshVanL JoshVanL requested review from a team as code owners July 20, 2023 17:36
Copy link
Contributor

@ItalyPaleAle ItalyPaleAle left a comment

Choose a reason for hiding this comment

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

LGTM to unblock builds, but can we also try to make the Windows tests work?

@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Merging #6704 (0517860) into master (7710be4) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #6704      +/-   ##
==========================================
+ Coverage   65.15%   65.20%   +0.04%     
==========================================
  Files         208      208              
  Lines       19388    19395       +7     
==========================================
+ Hits        12632    12646      +14     
+ Misses       5723     5712      -11     
- Partials     1033     1037       +4     
Impacted Files Coverage Δ
pkg/runtime/security/token.go 100.00% <ø> (ø)
pkg/http/middlewares.go 88.09% <100.00%> (+3.72%) ⬆️

... and 4 files with indirect coverage changes

@JoshVanL
Copy link
Contributor Author

LGTM to unblock builds, but can we also try to make the Windows tests work?

I agree. I would like to know why daprd seems to completely break down in serving on CPU constrained environments- assuming that is the issue. It doesn't seem windows OS specific to me.

@ItalyPaleAle
Copy link
Contributor

LGTM to unblock builds, but can we also try to make the Windows tests work?

I agree. I would like to know why daprd seems to completely break down in serving on CPU constrained environments- assuming that is the issue. It doesn't seem windows OS specific to me.

We know from prior experience that the Windows agents are slower. Not sure exactly why, but it may be related to the OS being heavier and the fact that it runs more complex anti-malware. Even I/O is significantly slower.

I also mentioned on Discord that I think we should change the ITs to run without the -race flag, since it makes tests much slower (and using more CPU), and it doesn't really help much in the case of ITs: since all binaries are pre-compiled, the flag can't help detecting race conditions there, and it can only detect race conditions in the test suite (which is of very little use IMHO).

@JoshVanL
Copy link
Contributor Author

I also mentioned on Discord that I think we should change the ITs to run without the -race flag, since it makes tests much slower (and using more CPU), and it doesn't really help much in the case of ITs: since all binaries are pre-compiled, the flag can't help detecting race conditions there, and it can only detect race conditions in the test suite (which is of very little use IMHO).

I'll open a PR to remove the -race flag and we can see what the result is.

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