tbeu / matio

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

Test reorganisation (results part) #29

Closed papadop closed 8 years ago

papadop commented 8 years ago

Here is a first step in tests reorganisation. I split all tests results in files (in test/results) and completely removed the use of MATIO_AT_HOST. You can find there are some small discrepancies between the output between the {4,5,7.3} versions. Most of them were removed with my previous patch, but some of them subsist. There are 3 types of discrepancies: 1) in 7.3 format logical are always int8 whereas in previous cases they can be of any type of int/uint, mat73.c has an explicit comment on this. 2) in dump, the order of variables differs with the format ? Is this normal ? 3) More strange: the difference between readvar-write_empty_struct-73-var4.out readvar-write_empty_struct-var4.out. This one looks like a genuine bug.

All such discrepancies can be seen with "ls -5 -73" and then compare those files with the ones with the -5 -73 removed.

A next step would be to remove the matlab test files (in test/matlab). I have some test failures when running the matlab tests. Is this normal ?

Then we can maybe generate the testsuite (.at) files from templates (I found some small discrepancies between the various {4,5,73} and {compressed,uncompressed}).

Anyway, this is already a huge improvement (in my view) and can be pulled independently, thus this pull request (a real one this time).

landscape-bot commented 8 years ago

Code Health Code quality remained the same when pulling 9991ac3 on papadop:NewTestsReorg into 31d6a37 on tbeu:master.

tbeu commented 8 years ago

Thanks for your PR.

I'll check the issues you observed.

As for the MATLAB test files and M scripts. I have no MATLAB on Linux available which can run these tests. So, it would be nice, if you are apparently able to run them, to open issues for them. They should not fail.

And yes moving them from the AT files to test/matlab is fine for me.

tbeu commented 8 years ago

Please add all files of test/results to the distribution. Thanks

papadop commented 8 years ago

I do not understand your last comment. The files have been added....

tbeu commented 8 years ago

No problem. I can do it.

I think I also found a fix for the EOL issue on Win.

tbeu commented 8 years ago

Why is it AT_CLEANUP(expout) in mat73_read_le.at but AT_CLEANUP in mat73_read_be.at? Which one to use?

tbeu commented 8 years ago

Files mat73_read_le.at and mat73_read_be.at miss AT_SKIP_IF([test $MAT73 -ne 1]) in line 578.

papadop commented 8 years ago

Well, as far as I understand AT_CLEANUP once had arguments and there were such calls. So I expected that AT_CLEANUP was clening the files in argument. It does not. So the proper call is just AT_CLEANUP. I intended to do that but forgot for some files. Consider it done with a next patch.

papadop commented 8 years ago

For the missing miss AT_SKIP_IF, I suspect they were already missing before. Unless I made a mistake, I did not touch anything with this respect.

tbeu commented 8 years ago

I already extended your changes. I can do it for these two follow-on changes.

tbeu commented 8 years ago

Your questions 1) in 7.3 format logical are always int8 whereas in previous cases they can be of any type of int/uint, mat73.c has an explicit comment on this. A: Need to check. 2) in dump, the order of variables differs with the format ? Is this normal ? A: No way to have it in same order with current MAT dataset files. Only renaming varx to var0x could help in proper sorting, but is not possible as long as little-endian MAT files cannot be regenerated. 3) More strange: the difference between readvar-write_empty_struct-73-var4.out readvar-write_empty_struct-var4.out. This one looks like a genuine bug. A: Need to check.

tbeu commented 8 years ago

Some tests are not yet reorganized. Are you doing it or did you left it for me?

papadop commented 8 years ago

I'm doing it.... But it's a little bit more difficult for me as matlab tests are so slow.... But progressing.... Note that some tests are currently blocking, I'll correct that.

tbeu commented 8 years ago

Third issue is fixed by 16bb5e80ca47c76905276d560756db4c9858f18d.

papadop commented 8 years ago

Great !!!