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

tests: make tests run in CI on windows #500

Merged
merged 3 commits into from
Apr 1, 2023

Conversation

OlivierLDff
Copy link
Contributor

@OlivierLDff OlivierLDff commented Mar 30, 2023

They were failing due to missing dll. This commit

  • inject the location of Qt dll in the PATH
  • Qt plugin directory in QT_PLUGIN_PATH
  • Qml import path in QML_IMPORT_PATH & QML2_IMPORT_PATH

They are not necessary to every tests, but at least they should be robust for the future when more tests will be added.

This commit use ENVIRONMENT_MODIFICATION instead of ENVIRONMENT, since adding a value to the PATH in windows is a mess otherwise with CMake, because ; is used as a list separator.

It also move add_subdirectory of book and example at the end of main CMakeLists.txt file, in order not to duplicate code to set RUNTIME_ENV.

Next step is to refactor all the add_test function in some helper function add_cxxqt_test or something like that.

fix #111

@Be-ing
Copy link
Contributor

Be-ing commented Mar 30, 2023

If you can get some of the tests to run but not all, feel free to add some back to the exclusion list. Any step forward would be helpful!

@OlivierLDff
Copy link
Contributor Author

Yes the goal of this pr is just to make test passed that are related to missing all. I removed all the filter for now to see what is working and what is not.

@OlivierLDff OlivierLDff force-pushed the fix-msvc-dll-path branch 16 times, most recently from 39e27c7 to 08b463d Compare March 31, 2023 19:00
@OlivierLDff OlivierLDff marked this pull request as ready for review March 31, 2023 20:17
Copy link
Contributor

@Be-ing Be-ing left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks so much. Just a few minor comments

CMakeLists.txt Show resolved Hide resolved
examples/qml_minimal/CMakeLists.txt Show resolved Hide resolved
@Be-ing
Copy link
Contributor

Be-ing commented Apr 1, 2023

Maybe we can revive #243 after merging this 🤔

@OlivierLDff OlivierLDff force-pushed the fix-msvc-dll-path branch 2 times, most recently from d4d46d4 to 0c13a84 Compare April 1, 2023 11:43
@OlivierLDff
Copy link
Contributor Author

I've force push a slight modification that also make valgrind test work with ninja multi config. So that every test can pass when using the CMakePresets.json from my other PR.

They were failing due to missing dll. This commit
- inject the location of Qt dll in the PATH
- Qt plugin directory in QT_PLUGIN_PATH
- Qml import path in QML_IMPORT_PATH & QML2_IMPORT_PATH

They are not necessary to every tests, but at least they should be robust for the future when more tests will be added.

This commit use `ENVIRONMENT_MODIFICATION` instead of `ENVIRONMENT`, since adding a value to the PATH in windows is a mess otherwise with CMake, because `;` is used as a list separator.

I took the liberty to refactor some boilerplate code, to make it working
more easily. More refactoring is needed to remove boilerplate code.

fixup!
@Be-ing Be-ing force-pushed the fix-msvc-dll-path branch from 83ddaf8 to 620ca02 Compare April 1, 2023 16:55
@Be-ing Be-ing enabled auto-merge (rebase) April 1, 2023 16:56
@Be-ing Be-ing merged commit cf32f77 into KDAB:main Apr 1, 2023
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.

Investigate why lots of tests are failing on Windows
2 participants