JuliaRheology / RHEOS.jl

RHEOS - Open Source Rheology data analysis software
MIT License
39 stars 9 forks source link

Fix Julia 1.7.3 warnings and errors #152

Closed moustachio-belvedere closed 1 year ago

moustachio-belvedere commented 2 years ago

When running the test suite on Julia 1.7.3 some new warnings and errors are raised.

Warnings where we should be using the more generic values and keys functions on kwargs. For example:

┌ Warning: use values(kwargs) and keys(kwargs) instead of kwargs.data and kwargs.itr
│   caller = #namedtuple#57 at symbols.jl:97 [inlined]
└ @ Core ~/.julia/packages/RHEOS/4cxTK/src/symbols.jl:97

Error arises from permissions bits on the test data files:

Error During Test at /home/--/.julia/packages/RHEOS/4cxTK/test/IO.jl:80
Test threw exception
Expression: _exportcsv_timestress()
SystemError: opening file
"/home/--/.julia/packages/RHEOS/4cxTK/test/testdata/testloop.csv": Permission denied

It seems that when you install the release version of RHEOS using ]add RHEOS all files have only read permission on Linux. Changing the permissions bits to be more flexible resolves this error. To fix this we could do a permissions test first and fail gracefully with a test skipped warning if it detects that the test data doesn't have the correct permissions. This is essentially a proxy test for whether the RHEOS version is from the Julia respository (public release) or from a git clone (for development).

akabla commented 1 year ago

thanks @moustachio-belvedere . I can't reproduce the error on 1.8. Is it still an issue for you?

I'll fix the warnings.

akabla commented 1 year ago

Sorry @moustachio-belvedere , I realise I did not test on the public release. So this most likely remains a problem. The issue is that we try to write on the package folder, which in hindsight seems inadequate.

I think a better solution might be to generate a random temporary file name for the test (in a suitable location in the computer), use it and delete the file when the test is complete.

I believe tempname() would return such a filename.

moustachio-belvedere commented 1 year ago

Tempname sounds like a good solution to me 👍

akabla commented 1 year ago

Tests are passing with the new temp file. I assume that it should now resolve the error, to be fully tested on the next public release.