Thank you for review, I'll take care of this in the coming days. About patch 3/8; It fixes compilation with C++ or MSVC. These tests are no-op when compiled as C with gcc/clang. For these tests and some other, I was thinking we could make them compile with C++ afterwards. I wanted to take care of this after we get tests working with the build system.
I not familiar with GitHub CI, but I can look into it. ________________________________ From: Martin Storsj? <[email protected]> Sent: Saturday, September 13, 2025 2:05 AM To: mingw-w64-public <[email protected]> Subject: Re: [Mingw-w64-public] winpthreads: integrate tests from tests_pthread directory with Automake On Fri, 29 Aug 2025, Kirill Makurin wrote: > This patch series integrates tests from `tests_pthread` subdirectory > with winpthread's build system, so that they can be run as part of > common `make` -> `make check` -> `make install` process or with `make > distcheck`. > > Some tests are failing. In order to not cause unexpected CI failures, I > marked them as XFAIL, but they probably need to be further investigated. > There are many things which can be updated and cleaned up in these > tests, but for now this should be good enough. Thanks for the series of patches! It looks mostly good, but I have a couple of smaller requests: - Many of the patches that fix up tests also contain unrelated cosmetic changes (removing trailing whitespace, or removing a trailing empty line at the end of the file and similar). This is quite distracting - can you split out all such cosmetic changes (and ideally also do the same change to other files that aren't touched at all) to a separate patch (either before or after the other changes)? - Patch 2/8, which moves 138 files, also contains functional changes to test.h (plus changed cosmetics). Can you split out those changes to a separate patch? (The removal of including ../src/mutex.h probably ideally is a preceding patch.) - Patch 3/8 fixes compilation of some tests - but there's no changes to XFAILs here (while other patches e.g. remove flags that disable errors/warnings when fixing them) - that's feels curious/odd. I see that e.g. exceptions/exception1.c is a no-op, not compiling the code that is fixed, unless the file is compiled as C++, or with MSVC. The current automake integration doesn't try to build them as C++, so they're effectively not built/executed, when run with mingw compilers here. So that should ideally be fixed as well, before/after these changes. - Patch 4/8, "fix compilation of some tests with recent gcc", also changes a number of cases of warnings of comparison between pointer and integer. That's the category of patch 5/8, so it'd be good to have those changes moved to that patch, to make it clearer what was an error. - Finally - it would be great if we'd execute these tests as part of the github CI integration as well. (I presume this is part of the end goal of these patches?) Can you make an attempt at adding such integration? I think it shouldn't be too hard to extend the existing code in the testcases-* jobs to enter the winpthreads directory and run "make check" there as well. // Martin _______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public _______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
