[test] Update WCOW uVM and vSMB, and HostProcess functional tests #1965
Conversation
helsaawy
force-pushed
the
func-tests
branch
2 times, most recently
from
241c958
to
f056672
Compare
helsaawy
changed the title
helsaawy
force-pushed
the
func-tests
branch
4 times, most recently
from
4c91058
to
2d896c5
Compare
helsaawy
force-pushed
the
func-tests
branch
2 times, most recently
from
f592f71
to
fcb7cc9
Compare
|
Seems like there are a bunch of changes combined in this PR, would it be possible to split it into multiple commits so that it is easier to review? |
helsaawy
force-pushed
the
func-tests
branch
3 times, most recently
from
bc56d8e
to
7e449aa
Compare
| if !errors.Is(errGive, err) { | ||
| tb.Fatalf("got stderr: %v; wanted: %v", errGive, err) | ||
| outGot, errGot := b.Output() | ||
| if !errors.Is(errGot, err) { |
There was a problem hiding this comment.
If errGot is say errors.New("file not found") will it match with os.ErrNotExist?
There was a problem hiding this comment.
nope, errors.Is checks that it matches the original error object, not the message itself
There was a problem hiding this comment.
Isn't that a problem then? Test could fail even when the actual error is what we want it to be?
There was a problem hiding this comment.
its by design, since if something doesnt exist, the function should return os.ErrNotExist (or fmt.Errorf("...: %w", os.ErrNotExist)), or whatever the agreed upon error is.
otherwise, we would have:
var ErrFileNotFound = errors.New("not found")
var ErrHandleNotFound = errors.New("not found")
errors.Is(ErrFileNotFound, ErrHandleNotFound)
also, (most of) hcsshim uses errors.Is instead of string matching (since the latter is bad practice and linters will complain), so this way tests match prod code
| out = strings.ToLower(strings.TrimSpace(out)) | ||
| outGot = strings.ToLower(strings.TrimSpace(outGot)) | ||
| if diff := cmp.Diff(out, outGot); diff != "" { | ||
| tb.Fatalf("stdout mismatch (-want +got):\n%s", diff) |
There was a problem hiding this comment.
stdout could be huge, probably should avoid logging the entire diff. Same for the function below.
There was a problem hiding this comment.
i tried that, but i found that whenever the test failed, having the stdout in the logs was pretty helpful to debug the failure immediately.
otherwise i went down a rabbit hole of replicating locally and then logging stdout anyways
There was a problem hiding this comment.
We can limit the size of the diff that we output. For example, if it is longer than 50 chars we only print first 50.
There was a problem hiding this comment.
can i defer that to a different PR? that would likely complicate this code unnecessarily
WCOW tests can be integrated directly into existing LCOW tests as subtests, after generalizing the original (LCOW-only) tests to run both types of uVMs and containers. Break the change into two: (1) move (and rename) the original LCOW-only tests; and (2) generalize the tests and add the WCOW components. To simplify the diffs, this commit only includes the first process. Specifically, move: - `lcow_bench_test.go` to `uvm_bench_test.go` - `lcow_container_test.go` to `container_test.go` - `lcow_test.go` to `lcow_uvm_test.go` Within `lcow_uvm_test.go`, combine and generalize kernel arg tests (i.e., `TestLCOW_UVMNoSCSINoVPMemInitrd` and `TestLCOW_UVMNoSCSISingleVPMemVHD`) to `TestLCOW_UVM_KernelArgs`. Combine and generalize boot/time tests (e.g., `TestLCOW_TimeUVMStartVHD`, `TestLCOW_UVMStart_KernelDirect_VHD`) to `TestLCOW_UVM_Boot`. Also, since go1.21, `"github.com/Microsoft/hcsshim/internal/sync"` is no longer necessary, so replace it with `"sync".OnceValue[s]`. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Expose `FileBindingSupported()` function from `"internal\jobcontainers"` so it can be used in functional testing code. Switch from `sync.Once` to checking for `bindfltapi.dll` during package init, since the check is (relatively) cheap. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Un-skip and fix WCOW uVM and container tests. Add WCOW: - uVM benchmarks - vSMB tests - Host Process tests For WCOW host process tests, add dedicated tests for setting username, and verifying hostname and volume mounts. Fix bug where removing a direct-mapped vSMB share fails. Run (non-virtualization/uVM) functional tests within CI. Starting Host Process containers requires SYSTEM to create a process with a specified token, so use PsExec.exe (from sysutils) to run tests. Make sure container specs are created with the default working directory (`C:\`), similar to how `internal\cmd` works). Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
|
rebased to resolve merge conflicts (update |
Update and un-skip WCOW uVM and container tests as well as WCOW vSMB tests.
Add:
Unified container and uVM lifecycle, exec, and IO tests for LCOW and WCOW.
Moved tests from:
lcow_bench_test.gotouvm_bench_test.golcow_container_test.gotocontainer_test.golcow_test.gotolcow_uvm_test.goanduvm_test.goFix bug where removing a direct-mapped vSMB share failes.
Run (non-virtualization/uVM) functional tests within CI.
Starting Host Process containers requires SYSTEM to create a process with a specified token, so use PsExec.exe (from sysutils) to run tests.
PR is split into three commits: the first does preparatory reorg work, specifically for LCOW-only tests that are generalized to work on both LCOW and WCOW in the last subsequent commit.