ci: combine go tests such that any test failure will report a failure for the entire task #3156
Conversation
QxBytes
added
ci
| cd ../platform/ | ||
| go test ./... | ||
| cd azure-container-networking/ | ||
| go test ./npm/... ./cni/... ./platform/... |
There was a problem hiding this comment.
I would add -race and -timeout as well.
There was a problem hiding this comment.
hmm I've tried adding -race but it seems to require CGO_ENABLED=1 (enabled), but on this particular node agent running the script (which is windows), it gives an error saying gcc is not found, making me believe this isn't possible at the moment, unless you have any other ideas of how to run -race? https://msazure.visualstudio.com/One/_build/results?buildId=108557696&view=logs&j=1e95c5bf-f785-5c1d-a4c1-33b038e882fb&t=4058cacc-5989-52ae-5341-5beff346fc28
There was a problem hiding this comment.
Ah, good to know it requires CGo. It is possible, and windows/amd64 is a supported GOOS/GOARCH for the race detector, however:
On Windows, the race detector runtime is sensitive to the version of the C compiler installed; as of Go 1.21, building a program with -race requires a C compiler that incorporates version 8 or later of the mingw-w64 runtime libraries
We can skip this for now, but CGo enablement will likely become required in the near future. The upside will be that we will be able to run the race detector once we're forced to turn CGo on.
I would still set a -timeout though.
There was a problem hiding this comment.
so if I'm understanding this correctly, the windows node at the moment doesn't have c compiler, and so we cannot have cgo enabled. In the future, we will need to somehow update the node agent to include gcc? In the meantime a -timeout has been added.
There was a problem hiding this comment.
No C compiler is required on the node for CGO_ENABLED=1. CGo means that some dependencies are dynamically linked (e.g. DNS resolution). As long as the DLLs for performing those functions are present on the system, it will work.
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
QxBytes
force-pushed
the
alew/add-set-ex-ut
branch
3 times, most recently
from
a16740d to
ac91b06
Compare
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
QxBytes
added
exempt-stale
|
This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days |
|
Pull request closed due to inactivity. |
QxBytes
force-pushed
the
alew/add-set-ex-ut
branch
from
6b4e7df to
35d4e37
Compare
|
/azp run Azure Container Networking PR |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Reason for Change:
When the go tests are split up, only the status of the last command run determines the status of the entire step/task. Any previous steps, even if they exited with a failure, won't cause the task to visibly fail. By combining all tests together in one command, if this single command fails (due to any of the tests failing), the step/task will visibly fail on the pipeline.
Issue Fixed:
See above
Requirements:
Notes:
Will fail in the pipeline until npm windows tests are fixed (this is expected, as previously those tests were silently failing).
When those npm changes are merged in, I will rebase this PR against master to resolve the errors.
Will be backported to
release/v1.5