tbeu / matio

MATLAB MAT File I/O Library
https://matio.sourceforge.io
BSD 2-Clause "Simplified" License
334 stars 97 forks source link

test suite on windows? #84

Closed agramfort closed 6 years ago

agramfort commented 6 years ago

hi,

we've been trying with @massich to migrate our build system to the matio conda packages for the OpenMEEG project.

So far we build everything from source using CMake external project mechanism but it makes our CI really slow and makes us maintain a lot of code. Also we would like to make soon a conda package based on the matio one.

while we thought we were almost there as it was working on mac and linux, we hit test errors on windows that only appear when we allow support for matlab IO. So it's matio related. See CIs status at the bottom of : https://github.com/massich/findmkl_openmeeg/pull/32

before digging into this we wanted to know if tests were run on windows by appveyor or some continuous integration? if someone tells us that all tests pass on windows then it's a bug in our code: https://github.com/massich/findmkl_openmeeg/blob/master/OpenMEEGMaths/include/MatlabIO.H that is specific to windows....

could there be any compiler definition / option that we may have missed on windows?

thanks a lot for any feedback or hint.

thanks

tbeu commented 6 years ago

I run the restsuite manually on my Win machine using MinGW (as it supports autotools). But it is not on Appveyor CI.

Sorry, I cannot see the error you are pointing to.

agramfort commented 6 years ago

so the tests are not run using visual studio?

let's see if we can provide with @massich a small example to replicate the windows pb we observed using the windows conda package.

tbeu commented 6 years ago

so the tests are not run using visual studio?

I do all the debugging and development on MSVC.

But you are right, before running the testsuite in MinGW I could replace the MinGW binaries by the MSVC binaries to check how it performs. Let's see.

agramfort commented 6 years ago

another question: why is there no matio_test_cases_compressed_hdf_be.mat?

tbeu commented 6 years ago

another question: why is there no matio_test_cases_compressed_hdf_be.mat?

Because I have no BE machine with MATLAB application. See #25.

tbeu commented 6 years ago

Sorry, the compressed one is called matio_test_cases_hdf_be.mat and is there.

agramfort commented 6 years ago

ok and this file corresponds to : matio_test_cases_uncompressed_hdf_le.mat or matio_test_cases_compressed_hdf_le.mat in be mode?

tbeu commented 6 years ago

File matio_test_cases_hdf_be.mat corresponds to matio_test_cases_compressed_hdf_le.mat in BE mode. Uncompressed HDF5 was first available with MATLAB R2017a. See #62.

agramfort commented 6 years ago

ok thanks. Any success running the tests with visual studio binaries?

tbeu commented 6 years ago

Any success running the tests with visual studio binaries?

Yes, 100% test success running the MSVC binaries through the testsuite in a Cygwin64 environment.

I noticed a small configuration improvement though, similar as c7805c970aa31625d25688905f727c0a5a22f901 and will fix the test for the diff command later on:

https://github.com/tbeu/matio/blob/5c2d85b68bf379fd179e7efdb8741a90f4ce17e2/test/testsuite.at#L55-L62

agramfort commented 6 years ago

ok then we'll have to dig into it but for now just changing in our code this line:

https://github.com/massich/findmkl_openmeeg/blob/master/OpenMEEGMaths/include/MatlabIO.H#L218

so it uses MAT_FT_DEFAULT and not MAT_FT_MAT73 makes our test suite pass using the windows conda packages... is it possible that we may need to change of usage of matio just changing this variable?

thanks a lot

massich commented 6 years ago

is there the possibility to add .appveyor.yml running the tests so that we can inspect and make sure we replicate properly?

We have created a cmake project to build matio without touching a single line of matio's codebase. But we don't seem to be able to include the testsuit without breaking the CI. (see this PR)

tbeu commented 6 years ago

I exported the appveyor.yml: appveyor.zip

agramfort commented 6 years ago

it says: Project not found or access denied.

tbeu commented 6 years ago

I know. But I attached the exported config.

massich commented 6 years ago

I see. If you upload this .appveyor.yml to my fork and do test: on won't work because you are testing using Cygwin64 in your local machine and to export the tests to MSVC is not feasible.

papadop commented 6 years ago

Alexandre and Joan, I'm working to integrate our OpenMEEG/matio cmake "fork" into the main matio repo. The new testsuite (when fully operational, which is not yet the case) will be able to run on windows. I hope to have a fully working prototype soon. @tbeu: I'm working on the testsuite to complete the work. This is also why I did (and still polishing) the autoconf simplifications (my newest branch), to make this a little bit easier.... So in the end, no codebase change will not totally be true. But minimal code base change (and common codebase for autoconf and cmake versions) is true.

tbeu commented 6 years ago

If you upload this .appveyor.yml to my fork and do test: on won't work because you are testing using Cygwin64 in your local machine and to export the tests to MSVC is not feasible.

True. This was only a local test I run for you after you asked for it. It is not part of any CI.

tbeu commented 6 years ago

is there the possibility to add .appveyor.yml running the tests so that we can inspect and make sure we replicate properly?

Check 7560022e33a43354192b5b3d836d9e405a86a0e2.

agramfort commented 6 years ago

thanks. So you are testing the 32bits version.

tbeu commented 6 years ago

Yes. Neither I succeeded in running the 64-bit build nor the MinGW build. Sorry for that. If you are luckier than me I would be glad to update.

agramfort commented 6 years ago

@massich maybe you can help here?

papadop commented 6 years ago

The cmake branch will probably help here.... Some more patience is needed as I have some personal problems right now. Give some more weeks to polish things up......

tbeu commented 4 years ago

Yes. Neither I succeeded in running the 64-bit build nor the MinGW build. Sorry for that. If you are luckier than me I would be glad to update.

Finally, test suite on Windows is available for cygwin, cygwin64 and mingw-w64 by 809af56b150f73e9cad590b256979ff528ea6b21.