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

[serialization] [tests] [java]ci.json creation obstructed by checkFoo unit tests #1062

Closed
izzyreal opened this issue Jul 29, 2023 · 1 comment

Comments

@izzyreal
Copy link

izzyreal commented Jul 29, 2023

Reproduction:

git clone --recursive https://github.com/kaitai-io/kaitai_struct.git && cd kaitai_struct

for module in $(git submodule status | awk '{print $2}')
do
    pushd $module
    git checkout serialization
    popd
done

cd tests && ./build-compiler && ./build-formats && ./ci-java

We get something like this:

/Users/izmar/git/tmp/kaitai_struct/tests/aggregate/junit_xml_parser.rb:37:in `block (2 levels) in each_test': Unable to parse name: "checkSizeMismatchCheckOrWrite" (RuntimeError)
	from /Library/Ruby/Gems/2.6.0/gems/rexml-3.2.5/lib/rexml/element.rb:1968:in `block in each'
	from /Library/Ruby/Gems/2.6.0/gems/rexml-3.2.5/lib/rexml/xpath.rb:68:in `each'
	from /Library/Ruby/Gems/2.6.0/gems/rexml-3.2.5/lib/rexml/xpath.rb:68:in `each'
	from /Library/Ruby/Gems/2.6.0/gems/rexml-3.2.5/lib/rexml/element.rb:1968:in `each'
	from /Users/izmar/git/tmp/kaitai_struct/tests/aggregate/junit_xml_parser.rb:21:in `block in each_test'
	from /Users/izmar/git/tmp/kaitai_struct/tests/aggregate/junit_xml_parser.rb:19:in `each'
	from /Users/izmar/git/tmp/kaitai_struct/tests/aggregate/junit_xml_parser.rb:19:in `each_test'
	from /Users/izmar/git/tmp/kaitai_struct/tests/aggregate/test_parser.rb:15:in `to_h'
	from aggregate/convert_to_json:77:in `<main>'

In junit_xml_parser.rb change

raise "Unable to parse name: \"#{name}\"" unless name =~ /^[Tt]est(.*?)$/

to

raise "Unable to parse name: \"#{name}\"" unless (name =~ /^[Tt]est(.*?)$/ || name =~ /^[Cc]heck(.*?)$/)

and we can cat test_out/java/ci.json again.

This is what I'm doing while working on #1060 (comment).

@generalmimon
Copy link
Member

generalmimon commented Jul 30, 2023

At the moment, I almost think the fact that the ci.json generation doesn't work for serialization tests for some reason is a good thing, because at least it's clear that our infrastructure isn't ready for them and some work is required to properly support them.

If we only do

raise "Unable to parse name: \"#{name}\"" unless (name =~ /^[Tt]est(.*?)$/ || name =~ /^[Cc]heck(.*?)$/)

...as you suggest, I guess this will remove the "obstruction" in the ci.json generation as you say, but the check* tests will be confused as test formats (for the reason I mentioned in #1060 (comment)), so I don't see much value in this hack.

In this case, though, calling the test methods just check*() without using test as a prefix is an unintentional mistake. These tests were written manually and TestNG technically doesn't require that the test methods start with test - it uses the @Test annotation to distinguish between test and non-test methods. However, unittest in Python for example requires that the tests methods start with test_, so I had to add the test_ prefix when porting the manual tests from Java to Python: kaitai-io/kaitai_struct_tests@6a3c0f1

So we should actually rename the check*() methods to testCheck*() in Java for consistency (with Python and with other tests in Java as well), but not really to "unobstruct" anything - the junit_xml_parser.rb ideally shouldn't care what the test methods are called.

@generalmimon generalmimon closed this as not planned Won't fix, can't repro, duplicate, stale Jul 30, 2023
generalmimon added a commit to kaitai-io/kaitai_struct_tests that referenced this issue Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants