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

try enabling tests on Windows #122

Closed
wants to merge 1 commit into from
Closed

try enabling tests on Windows #122

wants to merge 1 commit into from

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Apr 6, 2022

Maybe #105 fixed the link errors?

@Be-ing Be-ing marked this pull request as draft April 6, 2022 12:28
@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 6, 2022

Well that didn't work. I still don't know what's going wrong with those link errors.

@ahayzen-kdab
Copy link
Collaborator

Well that didn't work. I still don't know what's going wrong with those link errors.

Right :-/ I tried the same in a branch here earlier https://github.com/ahayzen-kdab/cxx-qt/commits/windows-exploration . I'll see if we can have someone with Windows knowledge spend some time trying to build the project locally and see if the cause can be found.

@LeonMatthesKDAB
Copy link
Collaborator

Just a random thought: We're in some places defining extern "C" functions that take references as arguments.

GCC has been warning about this for a while, as C doesn't have references, and we'd technically have to use pointers.
Maybe this is related somehow?

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 8, 2022

I don't see any compile warnings from GCC using GCC 11.2.1 locally on Fedora 35 nor on the Ubuntu CI.

@ahayzen-kdab
Copy link
Collaborator

IIRC the warnings were actually from clang and hence why we used to have this warning disable

#if defined(__clang__)

The Windows issues appear to be maybe linker related but it'll probably be more obvious on an actual machine than trying to fight CI, we just need to spend some time investigating what is missing/wrong.

@LeonMatthesKDAB
Copy link
Collaborator

Can this be closed?
We have Windows CI now, don't we?

@ahayzen-kdab
Copy link
Collaborator

Can this be closed? We have Windows CI now, don't we?

We don't there are problems ...

  • QML tests aren't being run
  • Cargo tests aren't being run
  • For me the examples crash on exit but not for Be (maybe a MSVC 2019 vs 2022 thing or something has fixed it)

@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 6, 2022

We still need to figure out what to do about adding Qt to PATH to fix the DLL not found errors when running the tests... or figure out some hack around #224.

@Be-ing
Copy link
Contributor Author

Be-ing commented Oct 6, 2022

However, the link errors with the tests have been fixed since overhauling the build system to compile the generated C++ with Cargo. The remaining issues are setting the required environment variables at runtime.

@ahayzen-kdab
Copy link
Collaborator

@Be-ing this can be closed now as there are working Windows tests? 🎉

@Be-ing
Copy link
Contributor Author

Be-ing commented Apr 7, 2023

Yes superseded by #500

@Be-ing Be-ing closed this Apr 7, 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.

3 participants