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

[test] Functional test housekeeping #1964

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

helsaawy
Copy link
Contributor

@helsaawy helsaawy commented Nov 8, 2023

Remove unused/legacy functional test flags/environment variables.

Unify TestMain control flow, so there is only one exit call, and defer is used to run cleanup after the tests are run.

Standardize UVM default[L|W]COWOptions to accept a context, and add context parameter to namespacedContext.

Remove all build tags aside from functional, since features are used to select the tests to run. This standardizes the functional tests with the cri-containerd tests, even though the feature names themselves are different.

Add the featureUVM to all tests that create a uVM, so that virtualization tests can be filtered out.

Add test/pkg/uvm.CreateWCOW function to mirror CreateLCOW, and add Create and CreateAndStart functions that pick LCOW or WCOW based on the options provided.

Have uVM scratch and image layers be created under a dedicated and persisted folder within %TEMP% that is excluded from Windows defender.
(The folder will be removed during OS restart, regardless of contents.)

Remove copied OCI spec options from test/internal/oci, add new options for creating HostProcess containers.

Add a internal\sync.OnceValue(Ctx) function that mirrors sync.OnceValues (introduced in go1.21) to have a type-safe Once function to use when initializing testing values.

Check that required privileges are held (only once) when unpacking Windows layers.

Fix LCOW tests in lcow_test.go that were setting KernelDirect without also updating KernelFile.

Standardize how (testing) binaries are built in the CI.

Relies on #1937

@helsaawy helsaawy added the tests Pull requests that modify tests label Nov 8, 2023
@helsaawy helsaawy requested a review from a team as a code owner November 8, 2023 20:24
@helsaawy helsaawy marked this pull request as draft November 8, 2023 20:24
@helsaawy helsaawy mentioned this pull request Jan 31, 2024
@helsaawy helsaawy marked this pull request as ready for review February 20, 2024 19:12
@helsaawy helsaawy force-pushed the func-test-cleanup branch from b7a9733 to 8bad8ea Compare March 5, 2024 19:01
Copy link
Contributor

@anmaxvl anmaxvl left a comment

Choose a reason for hiding this comment

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

lgtm.

@helsaawy helsaawy force-pushed the func-test-cleanup branch from 8bad8ea to 4aabbe2 Compare March 6, 2024 20:31
@@ -321,34 +327,45 @@ jobs:

# run tests
- name: Test repo
run: ${{ env.GOTESTCMD }} -gcflags=all=-d=checkptr -tags admin ./...
run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr -tags admin -timeout=20m ./...
Copy link
Contributor

Choose a reason for hiding this comment

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

why does only this one need a timeout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the default timeout of 10 minutes was cutting it close in some cases, i think

Copy link
Contributor

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

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

lgtm minus a few questions/nits

Remove unused/legacy functional test flags/environment variables.

Unify [TestMain] control flow, so there is only one exit call, and
`defer` is used to run cleanup after the tests are run.

Standardize UVM `default[L|W]COWOptions` to accept a context, and add
context parameter to `namespacedContext`

Remove all build tags aside from `functional`, since features are used
to select the tests to run. This standardizes the functional tests with
the cri-containerd tests, even though the feature names themselves are
different.

Add `test/pkg/uvm.CreateWCOW` function to mirror `CreateLCOW`, and add
`Create` and `CreateAndStart` functions that pick LCOW or WCOW based on
the options provided.

Have uVM scratch and image layers be created under a dedicated and
persisted folder within `%TEMP%` that is excluded from Windows defender.
(The folder will be removed during OS restart, regardless of contents.)

Remove copied OCI spec options from `test/internal/oci`, add new
options for creating HostProcess containers.

Add a `internal\sync.OnceValue`(`Ctx`) function that mirrors
`sync.OnceValues` (introduced in go1.21) to have a type-safe `Once`
function.

Check that required privileges are held (only once) when unpacking
Windows layers.

Fix LCOW tests in `lcow_test.go` that were setting `KernelDirect`
without also updating `KernelFile`.

Add `util.Context` function to create context that times out before test
timeout, to help with timing issues and allow time for cleanup and
logging.

Rename `cri_util` to `criutil`, since underscores are frowned upon in
package names.
Add a `test` prefix to `github.com/Microsoft/hcsshim/test/pkg/*` and
`github.com/Microsoft/hcsshim/test/internal/*` imports to be consistent
across all `test/functional/*` files.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
@helsaawy helsaawy force-pushed the func-test-cleanup branch from 4aabbe2 to 558c21b Compare March 7, 2024 16:36
@helsaawy helsaawy merged commit 523fe7b into microsoft:main Mar 7, 2024
19 checks passed
@helsaawy helsaawy deleted the func-test-cleanup branch March 7, 2024 20:54
ambarve pushed a commit to ambarve/hcsshim that referenced this pull request May 1, 2024
Remove unused/legacy functional test flags/environment variables.

Unify [TestMain] control flow, so there is only one exit call, and
`defer` is used to run cleanup after the tests are run.

Standardize UVM `default[L|W]COWOptions` to accept a context, and add
context parameter to `namespacedContext`

Remove all build tags aside from `functional`, since features are used
to select the tests to run. This standardizes the functional tests with
the cri-containerd tests, even though the feature names themselves are
different.

Add `test/pkg/uvm.CreateWCOW` function to mirror `CreateLCOW`, and add
`Create` and `CreateAndStart` functions that pick LCOW or WCOW based on
the options provided.

Have uVM scratch and image layers be created under a dedicated and
persisted folder within `%TEMP%` that is excluded from Windows defender.
(The folder will be removed during OS restart, regardless of contents.)

Remove copied OCI spec options from `test/internal/oci`, add new
options for creating HostProcess containers.

Add a `internal\sync.OnceValue`(`Ctx`) function that mirrors
`sync.OnceValues` (introduced in go1.21) to have a type-safe `Once`
function.

Check that required privileges are held (only once) when unpacking
Windows layers.

Fix LCOW tests in `lcow_test.go` that were setting `KernelDirect`
without also updating `KernelFile`.

Add `util.Context` function to create context that times out before test
timeout, to help with timing issues and allow time for cleanup and
logging.

Rename `cri_util` to `criutil`, since underscores are frowned upon in
package names.
Add a `test` prefix to `github.com/Microsoft/hcsshim/test/pkg/*` and
`github.com/Microsoft/hcsshim/test/internal/*` imports to be consistent
across all `test/functional/*` files.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
(cherry picked from commit 523fe7b)
Signed-off-by: Amit Barve <ambarve@microsoft.com>
princepereira pushed a commit to princepereira/hcsshim that referenced this pull request Aug 29, 2024
Remove unused/legacy functional test flags/environment variables.

Unify [TestMain] control flow, so there is only one exit call, and
`defer` is used to run cleanup after the tests are run.

Standardize UVM `default[L|W]COWOptions` to accept a context, and add
context parameter to `namespacedContext`

Remove all build tags aside from `functional`, since features are used
to select the tests to run. This standardizes the functional tests with
the cri-containerd tests, even though the feature names themselves are
different.

Add `test/pkg/uvm.CreateWCOW` function to mirror `CreateLCOW`, and add
`Create` and `CreateAndStart` functions that pick LCOW or WCOW based on
the options provided.

Have uVM scratch and image layers be created under a dedicated and
persisted folder within `%TEMP%` that is excluded from Windows defender.
(The folder will be removed during OS restart, regardless of contents.)

Remove copied OCI spec options from `test/internal/oci`, add new
options for creating HostProcess containers.

Add a `internal\sync.OnceValue`(`Ctx`) function that mirrors
`sync.OnceValues` (introduced in go1.21) to have a type-safe `Once`
function.

Check that required privileges are held (only once) when unpacking
Windows layers.

Fix LCOW tests in `lcow_test.go` that were setting `KernelDirect`
without also updating `KernelFile`.

Add `util.Context` function to create context that times out before test
timeout, to help with timing issues and allow time for cleanup and
logging.

Rename `cri_util` to `criutil`, since underscores are frowned upon in
package names.
Add a `test` prefix to `github.com/Microsoft/hcsshim/test/pkg/*` and
`github.com/Microsoft/hcsshim/test/internal/*` imports to be consistent
across all `test/functional/*` files.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Pull requests that modify tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants