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.

Sign up for GitHub

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

Jump to bottom

e2e fixes and changes #3823

Merged
merged 5 commits into from
Dec 20, 2022
Merged

e2e fixes and changes #3823

merged 5 commits into from
Dec 20, 2022

Conversation

phantomjinx
Copy link
Contributor

Release Note

NONE

tadayosi
tadayosi approved these changes Nov 18, 2022
View reviewed changes
Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

Great!

@tadayosi
Copy link
Member

@phantomjinx Is it too early yet to add windows-latest to the build workflow OS matrix?

@phantomjinx
Copy link
Contributor Author

@phantomjinx Is it too early yet to add windows-latest to the build workflow OS matrix?

Yeah. Lets give it a try (sorry misread you comment in my earlier reply).

This was referenced Nov 18, 2022
Closed
Closed
@phantomjinx
(e2e): Extend timeout for uninstall test
2687291 
* Wait a little longer to see if kamelets are successfully removed
@phantomjinx
(e2e): Prepares environment for windows OS support
1a205a4 
* Removes the dotenv action from e2e test suite. No longer required and
  does not support windows OS

* Assigns CLUSTER_TYPE in kamel-config-cluster. If not specified then default to
  cluster type of kind

* Conditional use of sudo - only for linux
@phantomjinx
(e2e): Fixes error in kustomize removal check
5379b40 
Stops exit 1 happening if kustomize not available
@tadayosi
Copy link
Member

tadayosi commented Dec 2, 2022

The install check failure is a known one which I'm already working on, and the local one is a temporary failure, so if you fix the validation failures then I think it's ready to merge.

@phantomjinx
Copy link
Contributor Author

phantomjinx commented Dec 6, 2022 •
edited
Loading

@phantomjinx Is it too early yet to add windows-latest to the build workflow OS matrix?

Yeah. Lets give it a try (sorry misread you comment in my earlier reply).

Tried enabling windows-latest on build workflow and it fails.

  1. Requires install of a distro in Windows Subsystem for Linux. So used:
   - name: Setup WSL (if on Windows)
      uses: Vampire/setup-wsl@v1.3.4
      if: runner.os == 'Windows'
  1. Fails to execute subshell scripts from make due to this issue: Unable to run bash scripts on a windows installed runner actions/runner#1328
./script/get_catalog.sh 1.16.0-SNAPSHOT 
/bin/bash: D:acamel-kcamel-kscriptget_catalog.sh: No such file or directory
mingw32-make: *** [Makefile:325: build-resources] Error 127
Error: Process completed with exit code 2.

@phantomjinx
Copy link
Contributor Author

Despite, the failure to initiate tests on windows, the changes should still be good. Can this be merged, please?

@tadayosi
Copy link
Member

@phantomjinx Yes, but there is validation failures that need to be fixed before merging. Could you update the pull req?

@phantomjinx
(e2e): make the use of paths os independent
fb04ff6 
* Uses FromSlash in tests to ensure comparisons on Windows are not affected
  by direction of slashes

* Uses filepath.Base as path.Base fails to return the final name when
  running on Windows

* Uses filepath.Join as path.Join, where appropriate, to return os
  corrected paths

* Tests / Resources
 * Uses filepath.ToSlash to make paths unix-like for consistent storage
   and easy comparison

* jvm
 * Ensure the classpath default paths are always converted to unix-style

* cmd/source/util.go
 * Handles difference between how linux and windows deal with an invalid
   path. Since a PathError is returned from the PermissionDenied test as
   well then handling this through the error message is the only alternative
@phantomjinx
(e2e): replaces deprecated set-output command
3755bcc 
* github has deprecated set-output function. Replaces with new syntax
@phantomjinx
Copy link
Contributor Author

@phantomjinx Yes, but there is validation failures that need to be fixed before merging. Could you update the pull req?

validation failures sorted.

@tadayosi tadayosi merged commit 0df10f3 into apache:main Dec 20, 2022
@tadayosi tadayosi mentioned this pull request Jan 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tadayosi tadayosi tadayosi approved these changes

@oscerd oscerd oscerd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@phantomjinx @tadayosi @oscerd