formats_err: add tests for issue #534 #124
Conversation
formats_err/imports_enum_leaking.ksy
Outdated
| @@ -0,0 +1,15 @@ | |||
| # ..\tests\formats_err/imports/enum_two.ksy: /seq/0/enum: | |||
There was a problem hiding this comment.
Should this be
| # ..\tests\formats_err/imports/enum_two.ksy: /seq/0/enum: | |
| # imports/enum_two.ksy: /seq/0/enum: |
?
Or
| # ..\tests\formats_err/imports/enum_two.ksy: /seq/0/enum: | |
| # ../tests/formats_err/imports/enum_two.ksy: /seq/0/enum: |
?
There was a problem hiding this comment.
The first option, because that's what you get on Unix-like systems. The fact that it's not the case on Windows can be considered a minor bug (although it's just a detail in the error message, so it's a bit annoying, but relatively harmless).
I also develop mostly on Windows (I run some things in WSL 2, but not all), so this sort of affects my local setup as well, but I'm relatively fine with ignoring a few local path-related test failures. If you want to resolve this properly, check out this commit of mine first, which will give you an explanation how these paths in formats_err error messages work: kaitai-io/kaitai_struct_compiler@5bc871a
The thing is that the Unix path would only contain forward slashes (like ../tests/formats_err/imports/enum_two.ksy: /seq/0/enum:), in which case ../tests/formats_err/ gets removed and only imports/enum_two.ksy: /seq/0/enum: remains. After taking a quick look, this apparently doesn't happen on Windows because of the way how yamlDir is computed here in KSC (JavaKSYParser.scala:21-22):
val yamlDir = Option(new File(yamlFilename).getParent).getOrElse(".")
val specs = new JavaClassSpecs(yamlDir, config.importPaths, firstSpec)The Java File class notoriously normalizes all paths to backslash-separated \ on Windows (more on that here: kaitai-io/kaitai_struct#507 (comment)), so it isn't very surprising to me that the value of yamlDir is ..\tests\formats_err even though yamlFilename was ../tests/formats_err/imports_enum_leaking.ksy.
The fix would be to use some other way to get the directory name of yamlFilename than new File(yamlFilename).getParent (as far as I know, the path normalization of Java File class can't be disabled, but there may be other APIs that don't do it). I'll just note that we shouldn't use any custom string manipulation for this (because we'd probably mess it up), but really try to find some existing reliable API in the standard library first.
There was a problem hiding this comment.
Ok, I removed the prefix and fix stripping on Windows in kaitai-io/kaitai_struct_compiler#304
Mingun
force-pushed
the
fix-type-leaking
branch
from
d98792e to
58ce2fe
Compare
There was a problem hiding this comment.
Please make all these lines at https://github.com/kaitai-io/kaitai_struct_tests/pull/124.patch disappear:
\ No newline at end of file
58ce2fe to
3b61772
Compare
I saw earlier that there probably was problems with processing of array types, when such errors inside them not reported. Add tests to ensure that they are being checked
Tests for kaitai-io/kaitai_struct#534 Adds new failing test in compiler: [info] - imports_type_leaking *** FAILED *** [info] [] [info] did not equal [info] [imports/type_two.ksy: /seq/0/type: [info] error: unable to find type 'type_one', searching from 'type_two' [info] ] (SimpleMatchers.scala:34)
Mingun
force-pushed
the
fix-type-leaking
branch
from
ada1791 to
e0a43e1
Compare
Tests for kaitai-io/kaitai_struct#534 Adds new failing test in compiler: [info] - imports_enum_leaking *** FAILED *** [info] [] [info] did not equal [info] [imports/enum_two.ksy: /seq/0/enum: [info] error: unable to find type 'enum_one', searching from 'enum_two' [info] [info] imports/enum_two.ksy: /instances/instance_one/enum: [info] error: unable to find type 'enum_one', searching from 'enum_two' [info] ] (SimpleMatchers.scala:34)
Mingun
force-pushed
the
fix-type-leaking
branch
from
e0a43e1 to
09ad33b
Compare
| # imports/enum_two.ksy: /seq/0/enum: | ||
| # error: unable to find type 'enum_one', searching from 'enum_two' | ||
| # | ||
| # imports/enum_two.ksy: /instances/instance_one/enum: | ||
| # error: unable to find type 'enum_one', searching from 'enum_two' | ||
| # |
There was a problem hiding this comment.
Strange, but the second error never reported in this test, even when I apply kaitai-io/kaitai_struct_compiler#313, so all errors should be reported. Don't figure yet, why
This PR adds tests for kaitai-io/kaitai_struct#534
Unfortunately, right now it contains Windows-specific paths which I'm not sure, should be completely removed or not? That is thy this PR in draft now
Related links: