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

Tests fail under Lua 5.4 #320

Closed
mcepl opened this issue Jul 7, 2020 · 8 comments · Fixed by #323
Closed

Tests fail under Lua 5.4 #320

mcepl opened this issue Jul 7, 2020 · 8 comments · Fixed by #323

Comments

@mcepl
Copy link

mcepl commented Jul 7, 2020

When running tests under 5.4, it fails with

[   37s] Running tests/test-import_into.lua
[   37s] Lua 5.4
[   37s] mod52.lua:13: assertion failed
[   37s] got:	"...abuild/rpmbuild/BUILD/Penlight-1.6.0/tests/lua/mod52.lua:14: attempt to call a nil value (global 'print')"
[   37s] needed:	"attempt to call global 'print'"
[   37s] these errors did not match
[   37s] Running tests/test-import_into.lua failed with code 1

The problem is obviously in https://github.com/Tieske/Penlight/blob/e469fa08b71055e1bc419f78754a1d830956e8d5/tests/lua/mod52.lua#L15 where the version of Lua is hardcoded.

I have created this patch

--- a/tests/lua/mod52.lua
+++ b/tests/lua/mod52.lua
@@ -12,7 +12,7 @@ function answer ()
     -- so define it as a local up above, or use utils.import(_G).
     test.assertraise(function()
         print 'hello'
-    end,(LUA_VERSION~="Lua 5.3") and "attempt to call global 'print'" or "attempt to call a nil value")
+    end,(LUA_VERSION~="Lua 5.3" and LUA_VERSION~="Lua 5.4") and "attempt to call global 'print'" or "attempt to call a nil value")
 
     -- but all the Penlight modules are available
     return pretty.write(utils.split '10 20  30', '')

but that’s obviously wrong solution, something more sophisticated should be created.

bmwiedemann added a commit to bmwiedemann/openSUSE that referenced this issue Jul 15, 2020
https://build.opensuse.org/request/show/819286
by user mcepl + dimstar_suse
- Add lua54.patch adding compatiblity with Lua 5.4
  (gh#lunarmodules/Penlight#320)

- Add lua54 as new build target
@anatol
Copy link

anatol commented Jul 19, 2020

I see the same issue with Arch Linux.

Should we use the patch from @mcepl until a better solution arrives?

@alerque
Copy link
Member

alerque commented Jul 21, 2020

Hey @Tieske this is a pretty minor thing but a major hold up for downsteam projects and distros. Lua 5.4 is starting to hit distros (Arch Linux, Homebrew, OpenSUSE, and probably more have active migration efforts in staging) and lua-penlight is becoming a hold up. It can be patched or built without checks, but it would be a lot easier if we could just fix this upstream hand them a release version that goes smoothly.

@Tieske
Copy link
Member

Tieske commented Jul 26, 2020

@alerque thx for the nudge, I'll have a look shortly

@Tieske
Copy link
Member

Tieske commented Jul 28, 2020

CI has been fixed, no code changes, just incompatible tooling.

Anything else needed for downstream distros? @mcepl @anatol

@mcepl
Copy link
Author

mcepl commented Jul 28, 2020

I don’t understand. Do you consider the fix when you just switch off testing on 5.4? (which is what #323 does, isn’t it?)

@Tieske
Copy link
Member

Tieske commented Jul 28, 2020

@mcepl

  • there was a hardcoded "5.3" in a test, which failed on 5.4, that test is fixed (no code change to the library)
  • luacov (test coverage) is disabled, but only on the 5.4 test run. Since there are no functional changes (see previous point) the coverage for 5.4 would be identical to 5.3 (which is still enabled)
  • luacheck (linter) is disabled, but only on the 5.4 test run. Since the sourcecode is the same, the linter results are also the same on each of the versions.

So overall 5.4 is being tested and passes the tests. Linting and coverage is disabled, but already covered by the tests across the other versions.

wrt reenabling those;

  • luacov needs fixing (mostly cluacov, luacov seems to work fine, but has a rockspec limitation defined ), see Lua 5.4 support luacov#83 (comment) for details what fails, and [DO NOT MERGE] Add Lua 5.4 support luarocks/cluacov#1 for some test/trial code (that doesn't work)
  • luacheck is currently unmaintained (the author passed away) so I do not expect an update there shortly. Also some initial testing I did indicated the tool is not 5.4 compatible.

So other than fixing that one test, there is nothing else to fix imo.

@mcepl
Copy link
Author

mcepl commented Jul 28, 2020

@Tieske Ah, makes sense, I haven’t noticed that it is only coverage and linter. Those don’t bother me at all. What about new release?

@Tieske
Copy link
Member

Tieske commented Jul 31, 2020

wokring my way through some issues, planning to do a new release one of these days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants