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

Fix test on Windows #19980

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

devhl-labs
Copy link
Contributor

@devhl-labs devhl-labs commented Oct 26, 2024

Fixes #19924

@joscha Pinging author of these tests for awareness

Here are the logs produced by ./mvnw clean package

[ERROR] Failures:
[ERROR] org.openapitools.codegen.DefaultCodegenTest.testExecutePostProcessor
[ERROR] Run 1: DefaultCodegenTest.testExecutePostProcessor:755 expected [true] but found [false]
[ERROR] Run 2: DefaultCodegenTest.testExecutePostProcessor:755 expected [true] but found [false]
[ERROR] Run 3: DefaultCodegenTest.testExecutePostProcessor:755 expected [true] but found [false]
[INFO]
[INFO]
[ERROR] Tests run: 3214, Failures: 1, Errors: 0, Skipped: 4

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh ./bin/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    (For Windows users, please run the script in Git BASH)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@joscha
Copy link
Contributor

joscha commented Oct 26, 2024

Can we find an execution that works for both windows and *nix or add an if/else in the test for the system instead of commenting it out?

@devhl-labs
Copy link
Contributor Author

devhl-labs commented Oct 26, 2024

I don't expect to troubleshoot this anytime soon but if you want to send a pr ill tell you if it works for me. Otherwise we should comment it out until we fix it. Also, maybe there should be a pr gate that runs the tests on windows.

@joscha
Copy link
Contributor

joscha commented Oct 26, 2024

Can you execute the commands behind call 2 and 3 on Windows and let me know what they yield, please? I do not have access to Windows unfortunately.

Agree with the CI step on Windows. I was under the impression this is something we do already.

@devhl-labs
Copy link
Contributor Author

devhl-labs commented Oct 26, 2024

They are all false.
image

Oh I see, stepping into it more I see this error. I tried changing echo to Write-Host and it still fails. Not sure what else to try.
"Cannot run program "echo": CreateProcess error=2, The system cannot find the file specified"

@joscha
Copy link
Contributor

joscha commented Oct 27, 2024

Is there no echo on modern windows anymore maybe? I think this needs Windows to debug it.

@wing328
Copy link
Member

wing328 commented Oct 27, 2024

We do have workflow to test on windows: https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/windows.yaml

mvn clean pacakge also works fine in my Windows box (JDK17)

@devhl-labs devhl-labs changed the title Comment out broken tests Fix test on Windows Oct 27, 2024
@devhl-labs
Copy link
Contributor Author

I fixed it

Copy link
Contributor

@joscha joscha left a comment

Choose a reason for hiding this comment

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

Thank you.

@wing328 wing328 merged commit 8916987 into OpenAPITools:master Oct 28, 2024
15 checks passed
@wing328 wing328 added this to the 7.10.0 milestone Oct 28, 2024
@devhl-labs devhl-labs deleted the 19924-comment-out-broken-test branch October 28, 2024 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] org.openapitools.codegen.DefaultCodegenTest#testExecutePostProcessor fails on Windows
3 participants