Some small test improvements#4427
Conversation
If you're adding `-v` to TESTFLAGS, you probably want to see the server logs, as well as the extra output from the testing framework. Signed-off-by: Paul "TBBle" Hampson <[email protected]>
Signed-off-by: Paul "TBBle" Hampson <[email protected]>
Signed-off-by: Paul "TBBle" Hampson <[email protected]>
|
Build succeeded.
|
| case 18363: // this isn't in osversion yet, but the image should be available | ||
| testImage = "mcr.microsoft.com/windows/nanoserver:1909" | ||
| case 19041: // this isn't in osversion yet, but the image should be available | ||
| testImage = "mcr.microsoft.com/windows/nanoserver:2004" | ||
| case 9200: // Missing manifest, so it's running in compatibility mode | ||
| fmt.Println("You need to copy Microsoft/hcsshim/test/functional/manifest/rsrc_amd64.syso into the containerd checkout") | ||
| panic("Running in Windows 8/Windows Server 2012 compatibility mode, failed to detect Windows build version") |
There was a problem hiding this comment.
Would be nice to have these added to https://github.com/microsoft/hcsshim/tree/master/osversion to give the versions a bit more meaning
There was a problem hiding this comment.
Wondering; would it work to use (e.g.) the official golang image for testing? It will be slightly larger (as it adds the go binaries), but that image is multi-arch, so should pull the right variant
There was a problem hiding this comment.
I'd prefer not to use a larger image. These tests already took ages to start on my own machine. -_- In the scheme of things, probably not a real problem to switch though, just me whinging. ^_^
Hmm. It's a shame there's not a multiarch nanoserver image, now I look. That would have been rather useful.
hcsshim already has Windows 2004... I just noticed it's missing 1909, so yeah, worth adding it there, and then when it's vendored next, these magic numbers can turn into named constants. Edit: I raised microsoft/hcsshim#857 to add a V19H2 constant.
9200 is trickier... It's slightly magic. It might be better to just add a copy of the syso, and/or work out how to generate it during the build. It apparently just appeared out of thin air one day which I guess means it's MIT License and can just be copied.
There was a problem hiding this comment.
I'd prefer not to use a larger image. These tests already took ages to start on my own machine. -- In the scheme of things, probably not a real problem to switch though, just me whinging. ^^
Yeah, the Windows images are huge. I think the Golang images are ~200M larger than the base images (which is a lot, but compared to the 2.5 .. 5.5 GB of the base image 😓 😂)
Hmm. It's a shame there's not a multiarch nanoserver image, now I look. That would have been rather useful.
There used to be, but Microsoft deprecated those in favour of users explicitly specifying the variant they need 😞
We could use @tianon's https://hub.docker.com/r/hell/win image (which is a multi-arch manifest referencing the upstream images) 😂
9200 is trickier... It's slightly magic. It might be better to just add a copy of the syso, and/or work out how to generate it during the build
Yes, I saw that version in moby/moby@9e587fa, which also mentions that's the version returned if the binary is not manifested.
Utility that's used in moby to get the correct version for that situation is here; https://github.com/moby/moby/blob/master/pkg/parsers/kernel/kernel_windows.go
There was a problem hiding this comment.
Thinking about it, if the _test file had something like import _ "github.com/Microsoft/hcsshim/test/functional/manifest", since we already vendor hcsshim, wouldn't that "just work"?
There was a problem hiding this comment.
I gave it a try (134e82505b78fc7a560d6ee7bd818c9d3dfd5e5f), and it seems to work. I had to use vndr -whitelist rsrc_amd64.syso as, for some reason I'm not clear on, vndr doesn't consider the .syso as used, even though it did pick up the manifest.go in the same directory. Maybe a bug in vndr? Edit: I raised LK4D4/vndr#94 to fix this, so once that flows through, we can make retry change.
And of course, that fails CI because when it runs vndr the .syso is deleted again. -_- So I'll drop the commit again, and we can try again elsewhere.
There was a problem hiding this comment.
Oh, yes, vndr has a list of known non-go files that it keeps (like header files), and other files will be removed. Should be easy to add this to that; https://github.com/LK4D4/vndr/blob/master/clean.go#L15-L50 feel free to open a PR there to add it (Alexander is usually fast to review/merge PR's)
There was a problem hiding this comment.
PR's already merged. If I understand correctly, it should be consumed by CI now, so I can try my previous commit again (in a new PR, since this one landed)
There was a problem hiding this comment.
yes, now that vndr has merged your PR, you can try again in a new PR
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
Some stuff I hit while trying to reproduce an integration test failure seen on CI:
-v, output the server log even on success, to help diagnose whatever you're trying to diagnose by adding-v.