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

Assorted BATS fixes for Windows #5160

Merged
merged 10 commits into from
Jul 20, 2023
Merged

Conversation

jandubois
Copy link
Member

No description provided.

@ericpromislow
Copy link
Contributor

Summary of failures:

platform.bats
 ✗ deploy s390x container
    in test file bats/tests/containers/platform.bats, line 62)
     `check_uname s390x s390x' failed

   -- output differs --
   expected (1 lines):
     s390x
   actual (2 lines):
     time="2023-07-13T22:30:04Z" level=warning msg="Platform \"linux/s390x\" seems incompatible with the host platform \"linux/amd64\". If you see \"exec format error\", see https://github.com/containerd/nerdctl/blob/main/docs/multi-platform.md"
     s390x
   --

On linux I just see s390x, but it looks like on Windows there's a warning line. Didn't investigate.

I'll write up each failure in a separate box to avoid making them too large.

@ericpromislow
Copy link
Contributor

switch-engines.bats

-- There were errors at

✗ switch to containerd 
✗ switch back to containerd and verify containers

but apparently these are expected

@ericpromislow
Copy link
Contributor

k8s/enable-disable-k8s

 ✗ re-enable kubernetes
   (from function `rdctl' in file bats/tests/helpers/commands.bash, line 72,
    in test file bats/tests/k8s/enable-disable-k8s.bats, line 27)
     `rdctl set --kubernetes-enabled=true' failed
   Error: Put "http://127.0.0.1:6107/v1/settings": dial tcp 127.0.0.1:6107: connectex: No connection could be made because the target machine actively refused it.

@ericpromislow
Copy link
Contributor

 preferences/list-settings-output
✗ generates registry output
 -- output does not contain substring --
   substring (1 lines):
     "pathManagementStrategy"="rcfiles"
...
  "pathManagementStrategy"="manual"

@ericpromislow
Copy link
Contributor

===== registry/creds ===== all tests fail with these error messages:

   (from function `local_setup' in file bats/tests/registry/creds.bats, line 21,
    from function `call_local_function' in file bats/tests/registry/../helpers/load.bash, line 68,
    from function `setup' in test file bats/tests/registry/../helpers/load.bash, line 98)
     `call_local_function' failed
   wslpath: /mnt/c/Users/eric/AppData/Local/Temp/auth: No such file or directory
   wslpath: /mnt/c/Users/eric/AppData/Local/Temp/certs: No such file or directory
   mkdir: cannot create directory ‘’: No such file or directory

Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

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

See failures in comments.

When using_windows_exe the path is converted to the windows path.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Invoke the full command via `sh -c "..."` instead.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Remove `assert_success` calls because we don't invoke `try` via `run`.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
It will fail with a non-zero exit code when not successful, so there is
no need to `assert_success` after calling `try` without `run`. And while
it does capture the command output in $output, that is an implementation
detail that tests should not rely on; they have to call `run try` to set
$output and $status.

Exiting with non-zero status was part of 431fa12 and outputting the
command output back to stdout was implemented in d678dba.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
It has a different default on Windows (manual) than on Unix (rcfiles).

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
@jandubois
Copy link
Member Author

@ericpromislow PTAL

With this PR only the switch-engines and traefik tests are known to fail, even when testing with containerd.

@ericpromislow
Copy link
Contributor

I'm still seeing #5160 (comment) -- this is using the Windows 1.9.1 release in user-mode, and the latest change on this branch

@ericpromislow
Copy link
Contributor

Same failure on k8s/enable-disable-k8s (again against the 1.9.1 release installed for me only)

@ericpromislow
Copy link
Contributor

My command-line:

RD_LOCATION=user RD_USE_WINDOWS_EXE=true bats/bats-core/bin/bats bats/tests/containers/platform.bats bats/tests/containers/switch-engines.bats  bats/tests/k8s/enable-disable-k8s.bats  bats/tests/preferences/list-settings-output.bats  bats/tests/registry/creds.bats

Only preferences/list-settings-output.bats passed this time. Everything else failed with the same error messages as in the previous run.

This was using this branch in a WSL session, with a 1.9.1 release installed for user only in Windows

Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

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

Some tests still fail. See my recent comments (2023-07-19 around 12 noon PDT)

@ericpromislow
Copy link
Contributor

Update using a newer CI build for RD fixed `containers/platforms.bats.

The registry/creds.bats needs to do mkdir -p "$AUTH_DIR" and mkdir -p "$CERTS_DIR" and not the ..._VOLUME variants. With that change, they passed.

Still failing:

containers/switch-engines.bats
k8s/enable-disable-k8s.bats

and I didn't run the traefik test which is known to fail

While the directories also exist on the host, they will not exist
in the WSL distro on Windows, so can't be created from there.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
It allows us to properly verify that a service has restarted.

This change does not fix the traefik test on Windows, but seems
like a more robust way to make sure we are using the restarted
cluster and not the original one.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

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

The two tests that are known to fail are still failing, and I saw a few network-related flakes. Approving so we can get these changes into main and make it easier to run win bats tests.

@jandubois jandubois merged commit abb2463 into rancher-sandbox:main Jul 20, 2023
@jandubois jandubois deleted the bats-windows branch July 20, 2023 21:13
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.

2 participants