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.

Sign up for GitHub

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

lua5.1 compatibility #89

Merged
merged 9 commits into from
Aug 13, 2020
Merged

lua5.1 compatibility #89

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5fbf646
lua5.1 compatibility
smarek Aug 10, 2020
dabf819
Lua 5.1 through 5.3 using ci-lua and ENV VARIETY set on CI, defaults …
smarek Aug 11, 2020
24cdf91
running Lua tests fixed
smarek Aug 12, 2020
ef47188
removed bash debug/verbose execution
smarek Aug 12, 2020
277930f
Codestyle
smarek Aug 13, 2020
c596605
Update ci-lua
smarek Aug 13, 2020
abfc758
revert lua version specific changes to tests
smarek Aug 13, 2020
abb1867
Update kst-adoption-report
smarek Aug 13, 2020
3cf26d3
simplify ci-lua
smarek Aug 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions ci-lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,42 @@

. ./config

rm -rf "$TEST_OUT_DIR/lua"
mkdir -p "$TEST_OUT_DIR/lua"
ALLOWED_LUA_VERSIONS='5.1 5.2 5.3'
CURRENT_LUA_VERSION=

# try to use $VARIETY from env
for allowed_version in $ALLOWED_LUA_VERSIONS; do
if [ "${allowed_version}" = "${VARIETY}" ]; then
CURRENT_LUA_VERSION="${VARIETY}"
fi
done

if [ -z "${CURRENT_LUA_VERSION}" ]; then
# fallback to version 5.1
CURRENT_LUA_VERSION="5.1"
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather use only lua command in this script (not lua${CURRENT_LUA_VERSION}). The ci-* scripts are not exactly designed for selecting the language compiler/interpreter like you did here. This should be done in .travis.yml ideally by installing the right apt packages or running some commands, eventually in prepare-lua script.

If running this (prepare-lua:13) will make available only the lua5.3 command and not lua (I don't know, that's a question):

hererocks lua_install -r^ -l${VARIETY}

Can you rather add an alias or symlink the lua5.3 executable (in .travis.yml build matrix or prepare-lua) to be available also as lua?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the Lua binary selection was already in place, I've only modified it to reflect on Travis env variable VARIETY
see https://github.com/kaitai-io/kaitai_struct_tests/blob/master/ci-lua#L13

If you want to change the way it was, i've just tested and hererocks always installs the lua into lua_install/bin/lua regardless of version in my environment. If the corect lua bin detection procedure was always pointless, we can safely get rid of that.

However, it might be useful for development if multiple lua binaries are installed, not through hererocks, just sayin

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to change the way it was, i've just tested and hererocks always installs the lua into lua_install/bin/lua regardless of version in my environment. If the corect lua bin detection procedure was always pointless, we can safely get rid of that.

Yeah, if that's the case, I'd definitely prefer it.

However, it might be useful for development if multiple lua binaries are installed, not through hererocks, just sayin

Well, I think it's easier to just temporarily symlink the lua command than rely on some magic variable, if one needs to run multiple Lua versions locally. Are the Lua versions 5.1, 5.2, and 5.3 really so different that it makes sense to test them all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, i'll change the script to only use lua binary, no detection whatsoever

They are different, especially 5.1 to 5.3 is big step, and 5.2 is the target we want because of Wireshark, so I'd really like to maintain high compatibility by testing with each version separately

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, comparison

5.3

./run-lua
Ran 210 tests in 0.044 seconds, 195 successes, 9 failures, 6 errors

5.2

./run-lua
Ran 210 tests in 0.038 seconds, 191 successes, 13 failures, 6 errors

5.1

./run-lua
Ran 210 tests in 0.038 seconds, 190 successes, 12 failures, 8 errors


LUA_OUT_DIR="${TEST_OUT_DIR}/lua"

rm -rf "${LUA_OUT_DIR}"
mkdir -p "${LUA_OUT_DIR}"

export LUA_PATH="$LUA_PATH;$LUA_RUNTIME_DIR/?.lua;spec/lua/?.lua;spec/lua/extra/?.lua;compiled/lua/?.lua;;"

# Add `lua_install` dir to PATH, as this is where hererocks installs Lua at CI
export PATH=$PATH:$PWD/../lua_install/bin

# Detect lua executable
if lua5.3 -v; then
LUA_BIN=lua5.3
if lua${CURRENT_LUA_VERSION} -v; then
LUA_BIN=lua${CURRENT_LUA_VERSION}
elif lua -v; then
LUA_BIN=lua
else
echo "Unable to detect lua executable, bailing out :("
exit 1
fi

"$LUA_BIN" spec/lua/run_test_suite.lua -v -o junit -n "$TEST_OUT_DIR/lua/report"
"$LUA_BIN" spec/lua/run_test_suite.lua --output junit --name "${LUA_OUT_DIR}/report"

./kst-adoption-report lua
aggregate/convert_to_json lua "$TEST_OUT_DIR/lua" "$TEST_OUT_DIR/lua/ci.json"
./kst-adoption-report "lua$"
aggregate/convert_to_json "lua" "${LUA_OUT_DIR}" "${LUA_OUT_DIR}/ci.json"
2 changes: 1 addition & 1 deletion kst-adoption-report
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def glob_spec_files(lang)
when 'javascript'
Dir.glob('spec/javascript/test_*.js')
when 'lua'
Dir.glob('spec/lua/test_*.lua')
Dir.glob("spec/lua/test_*.lua")
smarek marked this conversation as resolved.
Show resolved Hide resolved
when 'nim'
Dir.glob('spec/nim/tests/t*.nim')
when 'perl'
Expand Down
2 changes: 1 addition & 1 deletion run-lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@

export LUA_PATH="$LUA_PATH;$LUA_RUNTIME_DIR/?.lua;spec/lua/?.lua;spec/lua/extra/?.lua;compiled/lua/?.lua;;"

lua5.3 spec/lua/run_test_suite.lua -v
lua spec/lua/run_test_suite.lua -v
2 changes: 1 addition & 1 deletion spec/lua/run_test_suite.lua
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.