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

Test reorganisation (results part) #29

Merged
merged 3 commits into from
Mar 18, 2016
Merged

Conversation

papadop
Copy link
Contributor

@papadop papadop commented Mar 17, 2016

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
Copy link

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

@tbeu
Copy link
Owner

tbeu commented Mar 17, 2016

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
Copy link
Owner

tbeu commented Mar 17, 2016

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

@papadop
Copy link
Contributor Author

papadop commented Mar 17, 2016

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

@tbeu
Copy link
Owner

tbeu commented Mar 17, 2016

No problem. I can do it.

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

@tbeu
Copy link
Owner

tbeu commented Mar 18, 2016

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

@tbeu
Copy link
Owner

tbeu commented Mar 18, 2016

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

@papadop
Copy link
Contributor Author

papadop commented Mar 18, 2016

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
Copy link
Contributor Author

papadop commented Mar 18, 2016

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
Copy link
Owner

tbeu commented Mar 18, 2016

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

@tbeu tbeu merged commit 9991ac3 into tbeu:master Mar 18, 2016
@tbeu
Copy link
Owner

tbeu commented Mar 18, 2016

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
Copy link
Owner

tbeu commented Mar 19, 2016

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

@papadop
Copy link
Contributor Author

papadop commented Mar 26, 2016

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 added a commit that referenced this pull request Apr 11, 2016
* Yield same output as empty character array of HDF5 MAT file
* As reported by #29
tbeu added a commit that referenced this pull request Apr 11, 2016
* Yield same output as empty character array of HDF5 MAT file
* As reported by #29
@tbeu
Copy link
Owner

tbeu commented Apr 11, 2016

Third issue is fixed by 16bb5e8.

@papadop
Copy link
Contributor Author

papadop commented Apr 12, 2016

Great !!!

papadop pushed a commit to papadop/matio that referenced this pull request Nov 29, 2017
* Yield same output as empty character array of HDF5 MAT file
* As reported by tbeu#29
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.

None yet

3 participants