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

Julia #126

Merged
merged 8 commits into from
Jun 10, 2024
Merged

Julia #126

Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion aggregate/convert_to_json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ when 'lua', 'nim'
)
when 'rust'
JUnitXMLParser.new(infile).to_h
when 'go', 'perl', 'python', 'construct'
when 'go', 'perl', 'python', 'construct', 'julia'
add_kst_adoption(
JUnitXMLParser.new("#{infile}/report.xml").to_h,
infile
Expand Down
12 changes: 12 additions & 0 deletions aggregate/junit_xml_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ def each_test
elsif name =~ /^t(?!est)(.*?)$/
# Nim output
name = underscore_to_ucamelcase($1)
elsif tc.attribute('classname') and tc.attribute('classname').value =~ /^\/(.*?) test$/
# Julia output
name = $1
else
raise "Unable to parse name: \"#{name}\"" unless name =~ /^[Tt]est(.*?)$/
if $1[0] == '_'
Expand Down Expand Up @@ -71,6 +74,15 @@ def each_test

tr = TestResult.new(name, :skipped, 0, nil)
yield tr
elsif ts.attribute('errors') && ts.attribute('errors').value.to_f != 0 && ts.attribute('name').value =~ /^\/(.*?) test$/
Copy link
Member

@generalmimon generalmimon Jun 21, 2024

Choose a reason for hiding this comment

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

Parsing the errors attribute as a float (.to_f) doesn't make sense. It should be parsed as an integer, preferably using the strictest way possible, which would be Integer(..., 10) (see https://stackoverflow.com/a/49282/12940655).

Alternatively, we might not need to parse it at all, but instead can compare it directly to the string representation like ts.attribute('errors').value != '0' (this is even stricter, but all we really need, since there shouldn't be any other representations of zero anyway).

# Pick up Julia errored tests
name = $1
error_element = ts.elements['error']
error_message = error_element.attribute('message').value if error_element
error_trace = error_element.text.strip if error_element

tr = TestResult.new(name, :error, 0, TestResult::Failure.new(nil, nil, error_message, error_trace))
Copy link
Member

@generalmimon generalmimon Jun 21, 2024

Choose a reason for hiding this comment

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

This introduces a brand new test status error without any justification. We have never used this status, it is not supported by https://github.com/kaitai-io/kaitai_ci_ui and it is not clear what benefit (if any) it would bring. Until there is a discussion on this topic, it should be failed instead, because that is the status all other languages use in this case:

Suggested change
tr = TestResult.new(name, :error, 0, TestResult::Failure.new(nil, nil, error_message, error_trace))
tr = TestResult.new(name, :failed, 0, TestResult::Failure.new(nil, nil, error_message, error_trace))

yield tr
end
}
}
Expand Down
11 changes: 11 additions & 0 deletions ci-julia
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/sh

. ./config

rm -rf "$TEST_OUT_DIR/julia"
mkdir -p "$TEST_OUT_DIR/julia"

JULIA_LOAD_PATH="$JULIA_LOAD_PATH:$JULIA_RUNTIME_DIR:compiled/julia:spec/julia/extra" julia -e 'using Pkg; Pkg.activate("/env"); include("spec/julia/runtests.jl")' "$TEST_OUT_DIR/julia/report.xml"

./kst-adoption-report julia
aggregate/convert_to_json julia "$TEST_OUT_DIR/julia" "$TEST_OUT_DIR/julia/ci.json"
1 change: 1 addition & 0 deletions config
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ JAVA_RUNTIME_DIR=../runtime/java
JAVA_TESTNG_JAR=$HOME/.m2/repository/org/testng/testng/6.9.10/testng-6.9.10.jar:$HOME/.m2/repository/com/beust/jcommander/1.48/jcommander-1.48.jar
JAVASCRIPT_RUNTIME_DIR=../runtime/javascript
JAVASCRIPT_MODULES_DIR=node_modules
JULIA_RUNTIME_DIR=../runtime/julia
LUA_RUNTIME_DIR=../runtime/lua
NIM_RUNTIME_DIR=../runtime/nim
NIM_TESTIFY_DIR=spec/nim/testify
Expand Down
5 changes: 5 additions & 0 deletions kst-adoption-report
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def glob_spec_files(lang)
Dir.glob('spec/construct/**/test_*.py')
when 'ruby'
Dir.glob('spec/ruby/**/*_spec.rb')
when 'julia'
Dir.glob('spec/julia/test_*.jl')
else
raise "Unable to handle language #{@lang.inspect}"
end
Expand Down Expand Up @@ -74,6 +76,9 @@ def spec_file_to_test_name(lang, fn)
when 'ruby'
raise "Unable to extract test name from #{fn.inspect}" unless fn =~ /^(.*?)_spec\.rb$/
underscore_to_ucamelcase($1)
when 'julia'
raise "Unable to extract test name from #{fn.inspect}" unless fn =~ /^test_(.*?)\.jl$/
underscore_to_ucamelcase($1)
else
raise "Unable to handle language #{lang.inspect}"
end
Expand Down
10 changes: 10 additions & 0 deletions run-julia
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

This is the only run-* script with the #!/bin/bash shebang - all the other run-* scripts use mostly #!/bin/sh -ef. I don't see any reason to use #!/bin/bash here, given that the script apparently doesn't shouldn't have to use any bashisms. Using #!/bin/sh -ef instead will make it more portable, for example it will then work on Alpine Linux that doesn't have bash by default.

Edit: oh, source is actually a bashism, as ShellCheck points out:

Line 4:
source ./config
^-- SC3046 (warning): In POSIX sh, 'source' in place of '.' is undefined.

Well, that's another good reason to use . instead of source, as I suggest below.



Comment on lines +2 to +3
Copy link
Member

Choose a reason for hiding this comment

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

Double newline here:

Suggested change

source ./config
Copy link
Member

Choose a reason for hiding this comment

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

We use

Suggested change
source ./config
. ./config

everywhere.


# Define the command to run the Julia script with redirected output
command="JULIA_LOAD_PATH=\"$JULIA_LOAD_PATH:$JULIA_RUNTIME_DIR:compiled/julia:spec/julia/extra\" julia \"spec/julia/runtests.jl\""

# Run the command
eval $command
Comment on lines +6 to +10
Copy link
Member

Choose a reason for hiding this comment

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

I don't see any reason to use eval here. It makes everything ugly by having to escape the double quotes as \", and given that the $command in eval $command is unquoted, it is subject to all the globbing and word splitting as described in SC2086, which in this case means that JULIA_LOAD_PATH and JULIA_RUNTIME_DIR must not contain any spaces or glob characters, otherwise the script might break.

Why not just:

Suggested change
# Define the command to run the Julia script with redirected output
command="JULIA_LOAD_PATH=\"$JULIA_LOAD_PATH:$JULIA_RUNTIME_DIR:compiled/julia:spec/julia/extra\" julia \"spec/julia/runtests.jl\""
# Run the command
eval $command
JULIA_LOAD_PATH="$JULIA_LOAD_PATH:$JULIA_RUNTIME_DIR:compiled/julia:spec/julia/extra" julia spec/julia/runtests.jl

Also, I don't see the value of trivial comments such as Define the command to run the Julia script with redirected output (and I don't think this one actually makes sense - what exactly does "with redirected output" refer to? - I don't see any |, > or >> after the command, so it doesn't look like the output is redirected anywhere).

7 changes: 7 additions & 0 deletions spec/julia/extra/CustomFxNoArgs.jl

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

12 changes: 12 additions & 0 deletions spec/julia/extra/MyCustomFx.jl

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

13 changes: 13 additions & 0 deletions spec/julia/extra/Nested.jl

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

23 changes: 23 additions & 0 deletions spec/julia/runtests.jl

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

16 changes: 16 additions & 0 deletions spec/julia/test_bcd_user_type_be.jl

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

16 changes: 16 additions & 0 deletions spec/julia/test_bcd_user_type_le.jl

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

19 changes: 19 additions & 0 deletions spec/julia/test_bits_byte_aligned.jl

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

13 changes: 13 additions & 0 deletions spec/julia/test_bits_enum.jl

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

18 changes: 18 additions & 0 deletions spec/julia/test_bits_seq_endian_combo.jl

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

12 changes: 12 additions & 0 deletions spec/julia/test_bits_shift_by_b32_le.jl

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

12 changes: 12 additions & 0 deletions spec/julia/test_bits_shift_by_b64_le.jl

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

11 changes: 11 additions & 0 deletions spec/julia/test_bits_signed_res_b32_be.jl

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

11 changes: 11 additions & 0 deletions spec/julia/test_bits_signed_res_b32_le.jl

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

12 changes: 12 additions & 0 deletions spec/julia/test_bits_signed_shift_b32_le.jl

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

12 changes: 12 additions & 0 deletions spec/julia/test_bits_signed_shift_b64_le.jl

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

24 changes: 24 additions & 0 deletions spec/julia/test_bits_simple.jl

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

24 changes: 24 additions & 0 deletions spec/julia/test_bits_simple_le.jl

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

13 changes: 13 additions & 0 deletions spec/julia/test_bits_unaligned_b32_be.jl

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

13 changes: 13 additions & 0 deletions spec/julia/test_bits_unaligned_b32_le.jl

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

13 changes: 13 additions & 0 deletions spec/julia/test_bits_unaligned_b64_be.jl

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

Loading