Select Lua version #17
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
S-S-X
commented
Jan 15, 2024
action.yml
Outdated
| @@ -71,4 +75,4 @@ outputs: | |||
|
|
|||
| runs: | |||
| using: docker | |||
| image: Dockerfile | |||
| image: Dockerfile.${{ inputs.lua-version }} | |||
There was a problem hiding this comment.
Not sure if this work, depends if image: Dockerfile is some special trigger to start docker build instead of attempting docker pull.
Probably better to test before breaking tests of every project that is using this runner.
There was a problem hiding this comment.
Seems it isn't working as documented, here's even example that isn't far from this use case (and should be exactly same for expressions): https://docs.github.com/en/actions/creating-actions/creating-a-docker-container-action#creating-an-action-metadata-file
Failure is not with docker runner but with expression evaluation, either with inputs or github:
Unrecognized named-value: 'inputs'. Located at position 1 within expression: inputs.lua-version
and
Unrecognized named-value: 'github'. Located at position 1 within expression: github.event.inputs.lua-version
Coverage differences Lua 5.1 - LuaJITBecause of past compatibility issues Mineunit releases been using Busted 2.0. $ luacov-stats.sh luacov.busted-2.2.out
# Source files in report: 97/97
# Total coverage percent: 62.32%
$ luacov-stats.sh luacov.luajit.out
# Source files in report: 97/97
# Total coverage percent: 62.32%
$ luacov-stats.sh luacov.lua51.out
# Source files in report: 97/97
# Total coverage percent: 71.36%
$ grep '^\*\+0' luacov.lua51.out | wc -l
2244 # total misses in all source files
$ grep '^\*\+0' luacov.busted-2.2.out | wc -l
2564 # total misses in all source files
$ grep '^\*\+0' luacov.luajit.out | wc -l
2564 # total misses in all source filesMatching coverage for source files Lua 5.1 - LuaJITConsiders hits, misses and total coverage of lines with at least one hit.
Coverage differences for source files Lua 5.1 - LuaJITConsiders hits, misses and total coverage of lines with at least one hit.
Could maybe next take a bit closed look at it, where exactly are differences and is there something in common among them? |
|
LuaCov with LuaJIT 12 minetest.register_chatcommand("technic_get_active_networks", {
params = "[minlag]",
description = "Lists all active networks with additional network data",
12 privs = { [technic.config:get("admin_priv")] = true },
func = function(name, minlag)LuaCov with Lua 5.1 84 minetest.register_chatcommand("technic_get_active_networks", {
30 params = "[minlag]",
30 description = "Lists all active networks with additional network data",
54 privs = { [technic.config:get("admin_priv")] = true },
func = function(name, minlag)Reason for this seems to be simply that LuaJIT just isn't triggering the debug hook at all for some things. LuaCov still counts those "missed" lines and add those to totals so total coverage can only ever reach 100% in files where debug hooks get triggered at least once for every line that is also counted by LuaCov. For this reason LuaCov reports cannot really be compared between LuaJIT and Lua 5.1 runs. Fix: not so simple, either patch core of LuaCov or core of LuaJIT and hope it works. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
Most of previous release versions of Mineunit were shipped with Lua 5.1.
With latest 0.10 tried with LuaJIT again which at first seemed to work just fine.
However, turns out it doesn't.
Issues
While testing performace impact of new features I got around 30-40% test set failures with beerchat tests.
Interesting thing was that it seemed like if some stuff was executed in separate threads and resulted in race conditions, most of time just randomizing order of output like chat messages sent during tests. I suspected this might be some mistake with async stuff but there should not really be any async stuff there.
Seems this never happens with previous Lua 5.1 version and also not with 0.10 after switching implementation from LuaJIT to Lua 5.1.
Additionally it seems for some reason LuaCov is reporting way lower coverage for technic tests. For this have to investigate further to see if something in Mineunit actually gets disabled or is altered in such way that could affect execution of actual program or if it is LuaJIT / LuaCCov issue (possibly because of aggressive LuaJIT optimizations affecting debugger).
Weird LuaJIT behavior needs more investigation and preferably.
"solution"
Allow selecting Lua implementation with
lua-versionswitch. It accepts eitherluajitorlua51.Default is
lua51for now.Configuration examples
Example configuration to explicitly select LuaJIT:
Example configuration to explicitly select Lua 5.1:
Example configuration to use default Lua implementation (follow default setting of mt-mods/mineunit-actions):