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

Fix TranslatorSpec tests #273

Merged
merged 16 commits into from
Mar 8, 2024
Merged

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Mar 8, 2024

This PR fixes TranslatorSpec tests by changing expected values to the actually used.

Most commits fixes errors that was introduced during refactorings which did not update tests. This is understandable -- if the tests are already constantly falling, then few people think that they need to see if their changes break some of them.

Many tests related to that that each expression was wrapped in parenthesis even when this is not necessary. I know, the resulting code because of this can look ugly, and probably you want to remove those extra parenthesis instead of baking them into tests... but implementing such transalation is a long story and I think, it is better to fix tests now just so you can use normal workflow where other PRs will be accepted based on passed tests instead of blindly trust to the PR author and maintainers' reviews. I think everyone will agree that this has already led to a bunch of errors that have been hidden for years. If you want to remove those extra parenthesis, then open a separate issue / PR, where the changes in the code and in the tests will be made simultaneousionally.

As a result, after this PR is only 21 failed tests (61 before) and only 9 in TranslatorSpec due to missing implementations of various features. I recommend disable those tests until features would be implemented, but I don't know how to do that. Anyway, this is better to do in another PR.

@Mingun Mingun force-pushed the fix-translator-tests branch from 883ff24 to a7c48f0 Compare March 8, 2024 10:22
@Mingun Mingun marked this pull request as draft March 8, 2024 10:25
@Mingun Mingun force-pushed the fix-translator-tests branch from a7c48f0 to a14f87e Compare March 8, 2024 10:28
Mingun added 2 commits March 8, 2024 15:40
…to_i)

The behavior was changed in PR kaitai-io#106 (b889654)

Fixes

[info] - cpp_stl:some_bool.to_i *** FAILED ***
[info]   "[((some_bool()) ? 1 : 0])" was not equal to "[some_bool(])" (TranslatorSpec.scala:197)
[info]   Analysis:
[info]   "[((some_bool()) ? 1 : 0])" -> "[some_bool(])"
Fixes

[info] - cpp_stl:"12345".to_i *** FAILED ***
[info]   "[kaitai::kstream::string_to_int](std::string("12345"..." was not equal to "[std::stoi](std::string("12345"..." (TranslatorSpec.scala:520)
[info]   Analysis:
[info]   "[kaitai::kstream::string_to_int](std::string("12345"..." -> "[std::stoi](std::string("12345"..."
[info] - cpp_stl:"1234fe".to_i(16) *** FAILED ***
[info]   "[kaitai::kstream::string_to_int(std::string("1234fe")], 16)" was not equal to "[std::stoi(std::string("1234fe"), 0], 16)" (TranslatorSpec.scala:533)
[info]   Analysis:
[info]   "[kaitai::kstream::string_to_int(std::string("1234fe")], 16)" -> "[std::stoi(std::string("1234fe"), 0], 16)"
@Mingun Mingun force-pushed the fix-translator-tests branch from a14f87e to 06756a7 Compare March 8, 2024 11:37
Mingun added 4 commits March 8, 2024 16:50
…of byte arrays

Fixes

[info] - csharp:[0, 1, 2].length *** FAILED ***
[info]   no expected result, but actual result is new byte[] { 0, 1, 2 }.Length (TranslatorSpec.scala:332)
[info] - javascript:[0, 1, 2].length *** FAILED ***
[info]   no expected result, but actual result is [0, 1, 2].length (TranslatorSpec.scala:332)
[info] - lua:[0, 1, 2].length *** FAILED ***
[info]   "[#"\000\001\002"]" was not equal to "[string.len("str")]" (TranslatorSpec.scala:332)
[info]   Analysis:
[info]   "[#"\000\001\002"]" -> "[string.len("str")]"
…n tests for boolean expressions

It seems that Go always uses `this.UpperCase` instead of `lowerCase` field names
since it was introduced in 640809f.

Fixes

[info] - go:a != 2 and a != 5 *** FAILED ***
[info]   "[ ((this.A != 2) && (this.A != 5)) ]" was not equal to "[a != 2 && a != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ ((this.A != 2) && (this.A != 5)) ]" -> "[a != 2 && a != 5]"
It seems that it always translated to "\n" so it is strange why test expected "\u000a"

Fixes

[info] - go:"str\u000anext" *** FAILED ***
[info]   ""str\[n]next"" was not equal to ""str\[u000a]next"" (TranslatorSpec.scala:435)
[info]   Analysis:
[info]   ""str\[n]next"" -> ""str\[u000a]next""
@Mingun Mingun force-pushed the fix-translator-tests branch from 06756a7 to 62a9b6e Compare March 8, 2024 11:51
Mingun added 9 commits March 8, 2024 16:53
Most probably was done in f8dbedd

Fixes

[info] - go:"12345".to_i *** FAILED ***
[info]   "[tmp1, err := strconv.ParseInt("12345", 10, 0)
[info]   if err != nil {
[info]     return err
[info]   }
[info]   tmp1]" was not equal to "[func()(int){i, err := strconv.Atoi("12345"); if (err != nil) { panic(err) }; return i}()]" (TranslatorSpec.scala:520)
[info]   Analysis:
[info]   "[tmp1, err := strconv.ParseInt("12345", 10, 0)
[info] if err != nil {
[info]   return err
[info] }
[info] tmp1]" -> "[func()(int){i, err := strconv.Atoi("12345"); if (err != nil) { panic(err) }; return i}()]"
[info] - go:"1234fe".to_i(16) *** FAILED ***
[info]   "[tmp1, err := strconv.ParseInt("1234fe", 16, 0)
[info]   if err != nil {
[info]     return err
[info]   }
[info]   tmp1]" was not equal to "[func()(int64){i, err := strconv.ParseInt("1234fe", 16, 64); if (err != nil) { panic(err) }; return i}()]" (TranslatorSpec.scala:533)
[info]   Analysis:
[info]   "[tmp1, err := strconv.ParseInt("1234fe", 16, 0)
[info] if err != nil {
[info]   return err
[info] }
[info] tmp1]" -> "[func()(int64){i, err := strconv.ParseInt("1234fe", 16, 64); if (err != nil) { panic(err) }; return i}()]"
Changes was made in fa896a0

Fixes

[info] - go:a.last *** FAILED ***
[info]   "t[mp1 := this.A
[info]   tmp1[len(tmp1) - ]1]" was not equal to "t[his.A[len(this.A)-]1]" (TranslatorSpec.scala:382)
[info]   Analysis:
[info]   "t[mp1 := this.A
[info] tmp1[len(tmp1) - ]1]" -> "t[his.A[len(this.A)-]1]"
Changes was made in 4128d9f

Fixes

[info] - go:[].as<bytes> *** FAILED ***
[info]   "[[]uint8{}]" was not equal to "[""]" (TranslatorSpec.scala:588)
[info]   Analysis:
[info]   "[[]uint8{}]" -> "[""]"
[info] - go:[0 + 1, 5].as<bytes> *** FAILED ***
[info]   "[[]uint8{(0 + 1), 5}]" was not equal to "[string([]byte{(0 + 1), 5})]" (TranslatorSpec.scala:628)
[info]   Analysis:
[info]   "[[]uint8{(0 + 1), 5}]" -> "[string([]byte{(0 + 1), 5})]"
Fixes

[info] - java:_root.foo *** FAILED ***
[info]   "_root[()].foo()" was not equal to "_root[].foo()" (TranslatorSpec.scala:267)
[info]   Analysis:
[info]   "_root[()].foo()" -> "_root[].foo()"
…s implemented in 1f86742

Fixes

[info] - java:a[42 - 2] *** FAILED ***
[info]   "a().get(([int) (]42 - 2))" was not equal to "a().get(([]42 - 2))" (TranslatorSpec.scala:356)
[info]   Analysis:
[info]   "a().get(([int) (]42 - 2))" -> "a().get(([]42 - 2))"
…erals to represent characters in string

The change in translator was made in 338629d

Fixes

[info] - javascript:"str\0next" *** FAILED ***
[info]   ""str\[x]00next"" was not equal to ""str\[0]00next"" (TranslatorSpec.scala:448)
[info]   Analysis:
[info]   ""str\[x]00next"" -> ""str\[0]00next""
…that assume this

Fixes

[info] - lua:a[42] *** FAILED ***
[info]   "self.a[4[2 + 1]]" was not equal to "self.a[4[3]]" (TranslatorSpec.scala:343)
[info]   Analysis:
[info]   "self.a[4[2 + 1]]" -> "self.a[4[3]]"
[info] - lua:a[42 - 2] *** FAILED ***
[info]   "self.a[(4[2 - 2) + 1]]" was not equal to "self.a[(4[3 - 2)]]" (TranslatorSpec.scala:356)
[info]   Analysis:
[info]   "self.a[(4[2 - 2) + 1]]" -> "self.a[(4[3 - 2)]]"
…lements between 0 and 255

Fixes

[info] - python:[255, 0, 255] *** FAILED ***
[info]   "b"\[xFF\x00\xFF]"" was not equal to "b"\[255\000\255]"" (TranslatorSpec.scala:319)
[info]   Analysis:
[info]   "b"\[xFF\x00\xFF]"" -> "b"\[255\000\255]""
… all tests accordingly

Fixes

[info] - cpp_stl:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "([(2 < 3) ? (std::string("foo")) : (std::string("bar")]))" was not equal to "([2 < 3) ? (std::string("foo")) : (std::string("bar"]))" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "([(2 < 3) ? (std::string("foo")) : (std::string("bar")]))" -> "([2 < 3) ? (std::string("foo")) : (std::string("bar"]))"
[info] - csharp:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "[(2 < 3 ? "foo" : "bar")]" was not equal to "[2 < 3 ? "foo" : "bar"]" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "[(2 < 3 ? "foo" : "bar")]" -> "[2 < 3 ? "foo" : "bar"]"
[info] - java:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "[(2 < 3 ? "foo" : "bar")]" was not equal to "[2 < 3 ? "foo" : "bar"]" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "[(2 < 3 ? "foo" : "bar")]" -> "[2 < 3 ? "foo" : "bar"]"
[info] - javascript:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "[(2 < 3 ? "foo" : "bar")]" was not equal to "[2 < 3 ? "foo" : "bar"]" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "[(2 < 3 ? "foo" : "bar")]" -> "[2 < 3 ? "foo" : "bar"]"
[info] - lua:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "utils.box_unwrap([(2 < 3) and utils.box_wrap("foo") or ("bar")])" was not equal to "utils.box_unwrap([2 < 3 and utils.box_wrap("foo") or "bar"])" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "utils.box_unwrap([(2 < 3) and utils.box_wrap("foo") or ("bar")])" -> "utils.box_unwrap([2 < 3 and utils.box_wrap("foo") or "bar"])"
[info] - perl:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "[(2 < 3 ? "foo" : "bar")]" was not equal to "[2 < 3 ? "foo" : "bar"]" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "[(2 < 3 ? "foo" : "bar")]" -> "[2 < 3 ? "foo" : "bar"]"
[info] - php:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "[(2 < 3 ? "foo" : "bar")]" was not equal to "[2 < 3 ? "foo" : "bar"]" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "[(2 < 3 ? "foo" : "bar")]" -> "[2 < 3 ? "foo" : "bar"]"
[info] - python:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "[(u"foo" if 2 < 3 else u"bar")]" was not equal to "[u"foo" if 2 < 3 else u"bar"]" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "[(u"foo" if 2 < 3 else u"bar")]" -> "[u"foo" if 2 < 3 else u"bar"]"
[info] - ruby:2 < 3 ? "foo" : "bar" *** FAILED ***
[info]   "[(2 < 3 ? "foo" : "bar")]" was not equal to "[2 < 3 ? "foo" : "bar"]" (TranslatorSpec.scala:133)
[info]   Analysis:
[info]   "[(2 < 3 ? "foo" : "bar")]" -> "[2 < 3 ? "foo" : "bar"]"

[info] - cpp_stl:some_bool.to_i *** FAILED ***
[info]   "[((some_bool()) ? 1 : 0])" was not equal to "[some_bool(])" (TranslatorSpec.scala:197)
[info]   Analysis:
[info]   "[((some_bool()) ? 1 : 0])" -> "[some_bool(])"
[info] - lua:some_bool.to_i *** FAILED ***
[info]   "[(self.some_bool and 1 or 0)]" was not equal to "[self.some_bool and 1 or 0]" (TranslatorSpec.scala:197)
[info]   Analysis:
[info]   "[(self.some_bool and 1 or 0)]" -> "[self.some_bool and 1 or 0]"

[info] - cpp_stl:a != 2 and a != 5 *** FAILED ***
[info]   "[ ((a() != 2) && (a() != 5)) ]" was not equal to "[a() != 2 && a() != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ ((a() != 2) && (a() != 5)) ]" -> "[a() != 2 && a() != 5]"
[info] - csharp:a != 2 and a != 5 *** FAILED ***
[info]   "[ ((A != 2) && (A != 5)) ]" was not equal to "[A != 2 && A != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ ((A != 2) && (A != 5)) ]" -> "[A != 2 && A != 5]"
[info] - java:a != 2 and a != 5 *** FAILED ***
[info]   "[ ((a() != 2) && (a() != 5)) ]" was not equal to "[a() != 2 && a() != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ ((a() != 2) && (a() != 5)) ]" -> "[a() != 2 && a() != 5]"
[info] - javascript:a != 2 and a != 5 *** FAILED ***
[info]   "[ ((this.a != 2) && (this.a != 5)) ]" was not equal to "[this.a != 2 && this.a != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ ((this.a != 2) && (this.a != 5)) ]" -> "[this.a != 2 && this.a != 5]"
[info] - lua:a != 2 and a != 5 *** FAILED ***
[info]   "[ ((self.a ~= 2) and (self.a ~= 5)) ]" was not equal to "[self.a ~= 2 and self.a ~= 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ ((self.a ~= 2) and (self.a ~= 5)) ]" -> "[self.a ~= 2 and self.a ~= 5]"
[info] - perl:a != 2 and a != 5 *** FAILED ***
[info]   "[ (($self->a() != 2) && ($self->a() != 5)) ]" was not equal to "[$self->a() != 2 && $self->a() != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ (($self->a() != 2) && ($self->a() != 5)) ]" -> "[$self->a() != 2 && $self->a() != 5]"
[info] - php:a != 2 and a != 5 *** FAILED ***
[info]   "[ (($this->a() != 2) && ($this->a() != 5)) ]" was not equal to "[$this->a() != 2 && $this->a() != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ (($this->a() != 2) && ($this->a() != 5)) ]" -> "[$this->a() != 2 && $this->a() != 5]"
[info] - python:a != 2 and a != 5 *** FAILED ***
[info]   "[ ((self.a != 2) and (self.a != 5)) ]" was not equal to "[self.a != 2 and self.a != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ ((self.a != 2) and (self.a != 5)) ]" -> "[self.a != 2 and self.a != 5]"
[info] - ruby:a != 2 and a != 5 *** FAILED ***
[info]   "[ ((a != 2) && (a != 5)) ]" was not equal to "[a != 2 && a != 5]" (TranslatorSpec.scala:280)
[info]   Analysis:
[info]   "[ ((a != 2) && (a != 5)) ]" -> "[a != 2 && a != 5]"

[info] - python:"str".reverse *** FAILED ***
[info]   "[(u"str")][::-1]" was not equal to "[u"str"][::-1]" (TranslatorSpec.scala:507)
[info]   Analysis:
[info]   "[(u"str")][::-1]" -> "[u"str"][::-1]"
@Mingun Mingun force-pushed the fix-translator-tests branch from 62a9b6e to 30c0201 Compare March 8, 2024 11:54
@Mingun Mingun marked this pull request as ready for review March 8, 2024 12:01
@GreyCat
Copy link
Member

GreyCat commented Mar 8, 2024

Thanks for this big effort! I believe it's indeed better to merge this first, and then I wanted to work finally on kaitai-io/kaitai_struct#69 — this will require a lot of reverts of these, but looks like it's worth it.

@GreyCat GreyCat merged commit d071323 into kaitai-io:master Mar 8, 2024
1 of 4 checks passed
@Mingun Mingun deleted the fix-translator-tests branch March 8, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants