cucumber-cpp icon indicating copy to clipboard operation
cucumber-cpp copied to clipboard

adding windows test driver fix

Open kreuzberger opened this issue 10 months ago • 10 comments

Summary

Fix #292

Details

Different QtTestDriver Implemenation to ensure no resource lock on windows. Issue with undefined output format for qExec, add explicitly ".txt" extension

Motivation and Context

Fix #292

How Has This Been Tested?

Running internal tests with/without changes on ubuntu 22.04 and win10 vs2019

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue).
  • [ ] New feature (non-breaking change which adds functionality).
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • [x] It is my own work, its copyright is implicitly assigned to the project and no substantial part of it has been copied from other sources (including Stack Overflow). In rare occasions this is acceptable, like in CMake modules where the original copyright information should be kept.
  • [ ] I'm using the same code standards as the existing code (indentation, spacing, variable naming, ...).
  • [ ] I've added tests for my code.
  • [x] I have verified whether my change requires changes to the documentation
  • [x] My change either requires no documentation change or I've updated the documentation accordingly.
  • [x] My branch has been rebased to main, keeping only relevant commits.

kreuzberger avatar Apr 08 '24 08:04 kreuzberger

Due to the checks: Could you provide a clang-format file (.clang-format) or what should be used for clang-format as default style? Assume clang-format without any further options?

kreuzberger avatar Apr 10 '24 19:04 kreuzberger

Due to the checks: Could you provide a clang-format file (.clang-format) or what should be used for clang-format as default style? Assume clang-format without any further options?

The format file is checked in: https://github.com/cucumber/cucumber-cpp/blob/main/.clang-format Clang-format may produce slightly different out depending on the version. It usually is no problem.

ursfassler avatar Apr 11 '24 06:04 ursfassler

Current Status

TLDR: Windows Tests integration, would only merge after some clarifications

  • [x] Tried to integrate the issues from the reviews
    • [x] Qt5 and Qt6 differ in QTemporaryFile Name generation if its template based. Added applicationame and pid in a reproducable way (hope so :smile:)
    • [x] Enabled file removal. Might be an issue to change.
    • [x] Suggested early exit handling due to readability
    • [x] clang formating
  • [x] Fixed run-linux.sh issue to use Qt6 and disable Qt5 if both are found
  • [x] Refactored run-windows powershell script to
    • [x] Build Release configuration only (Could Ninja be used? works, but do not know status on test vm)
    • [x] Run cmake tests
    • [x] Add install step like for linux
    • [x] Build examples and run example test executables for GTest
    • [ ] Build examples and run example test executables for QtTest (if qmake found in path, better solution for getting the Qt Runtime Path for windows would be acceptable.)
    • [ ] Sometimes (often) failing windows Tests with QtTestDriver
  • [x] Integrate the suggested QTemporayFileWrapper

Open Issues

On Windows the QtTestDriver failes sometimes. Reason is that in one test the Text Error messages are compared. Due to the nature of the default QtTest output the diff test fails cause the Test execution time differs. This could also occur on linux, but seems not so often there (currently im running windows in VM)

+ Totals: 2 passed, 1 failed, 0 skipped, 0 blacklisted, 2ms
+ ********* Finished testing of cucumber::internal::QtTestObject *********
- Totals: 2 passed, 1 failed, 0 skipped, 0 blacklisted, 1ms
- ********* Finished testing of cucumber::internal::QtTestObject *********

Dont know how to fix.

For the QtTests to work which are not part of ctest the Qt Runtime Path (bin on Windows) must be in PATH. Do currently not know HOW to get this in the CI. Workarounded by having it in the PATH env of the caller. This could be fixed running these Tests in CMake with the runtime Path determined like for the other ctests and let the test code (starting and waiting for the processes) be run with os dependent scripts.

kreuzberger avatar Apr 11 '24 14:04 kreuzberger

Hi! Would it be possible to let the checks run on commit, so i can see open issues and fix them?

kreuzberger avatar Apr 15 '24 18:04 kreuzberger

On Windows the QtTestDriver failes sometimes. Reason is that in one test the Text Error messages are compared. Due to the nature of the default QtTest output the diff test fails cause the Test execution time differs.

:astonished: That would be a bad tests, timings shouldn't be compared in the tests. What test is it?

ursfassler avatar Apr 15 '24 19:04 ursfassler

Fixed run-linux.sh issue to use Qt6 and disable Qt5 if both are found

Idea is that only the Qt version you specified is searched for.

ursfassler avatar Apr 15 '24 19:04 ursfassler

On Windows the QtTestDriver failes sometimes. Reason is that in one test the Text Error messages are compared. Due to the nature of the default QtTest output the diff test fails cause the Test execution time differs.

😲 That would be a bad tests, timings shouldn't be compared in the tests. What test is it?

QtTestDriverTest, see open issues from above

kreuzberger avatar Apr 15 '24 20:04 kreuzberger

@kreuzberger please see my PR #296 to fix the flaky test.

I also investigated if it is possible to capture the test log without needing the file. Unfortunately it seems not to be possible (without too much hacking). Meaning that this PR is quite relevant.

ursfassler avatar Apr 29 '24 12:04 ursfassler

@kreuzberger please see my PR #296 to fix the flaky test.

I also investigated if it is possible to capture the test log without needing the file. Unfortunately it seems not to be possible (without too much hacking). Meaning that this PR is quite relevant.

Commented your PR #296.

kreuzberger avatar May 02 '24 14:05 kreuzberger

@kreuzberger unfortunetaly the Windows build has a problem with gtest.

@cwellm did you also had this problem? Could you also help out with reviewing the windows code?

ursfassler avatar May 09 '24 07:05 ursfassler