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

Do Scala.js properly #579

Closed
wants to merge 26 commits into from
Closed

Do Scala.js properly #579

wants to merge 26 commits into from

Conversation

sideeffffect
Copy link
Member

@sideeffffect sideeffffect requested a review from a team as a code owner June 29, 2023 21:14
@sideeffffect
Copy link
Member Author

Hello @thinkharderdev @vigoo , I'm trying to port zio-schema to Scala.js.
I think I've overcome the simple technical obstacles that are typical for this porting effort. But now I'm faced with flat out failing tests in JS and I have no idea why the JS code should behave any differently from the JVM code to cause tests to fail. Could you please have a look? Maybe it's something obvious that I'm missing since I'm not familiar with zio-schema and it's code and principles of working.

@vigoo
Copy link
Contributor

vigoo commented Nov 18, 2023

@sideeffffect I merged the latest updates to this, and now I see some cross-platform related failures I don't know much about:

[error]    |  /home/vigoo/.cache/coursier/v1/https/repo1.maven.org/maven2/org/portable-scala/portable-scala-reflect_sjs1_2.13/1.1.2/portable-scala-reflect_sjs1_2.13-1.1.2.jar(org/portablescala/reflect/annotation/package.class)
[error]    |and also in
[error]    |  /home/vigoo/.cache/coursier/v1/https/repo1.maven.org/maven2/org/portable-scala/portable-scala-reflect_2.13/1.1.2/portable-scala-reflect_2.13-1.1.2.jar(org/portablescala/reflect/annotation/EnableReflectiveInstantiation.class)

could you take a look?

@sideeffffect
Copy link
Member Author

Hi, I got it to the state where things compile and tests run. But they fail, there are many errors in the style

✗ true was not equal to false

Could you please have a look at this? Unfortunately I don't understand zio-schema very well, so I don't have much clue what could be going wrong 😅

Some of them also look like this

[info]     - Schema Spec / Should have valid equals / sequence
[info]       ✗ Sequence(
[info]           elementSchema = Primitive(
[info]             standardType = unit,
[info]             annotations = Chunk()
[info]           ),
[info]           fromChunk = <function1>,
[info]           toChunk = <function1>,
[info]           annotations = Chunk(),
[info]           identity = "Chunk"
[info]         ) was not equal to Sequence(
[info]           elementSchema = Primitive(
[info]             standardType = unit,
[info]             annotations = Chunk()
[info]           ),
[info]           fromChunk = <function1>,
[info]           toChunk = <function1>,
[info]           annotations = Chunk(),
[info]           identity = "Chunk"
[info]         )
[info]       Schema.chunk(schemaUnit) did not satisfy equalTo(Schema.chunk(schemaUnit))
[info]       Schema.chunk(schemaUnit) = Sequence(
[info]         elementSchema = Primitive(
[info]           standardType = unit,
[info]           annotations = Chunk()
[info]         ),
[info]         fromChunk = <function1>,
[info]         toChunk = <function1>,
[info]         annotations = Chunk(),
[info]         identity = "Chunk"
[info]       )
[info]       at /home/runner/work/zio-schema/zio-schema/tests/shared/src/test/scala/zio/schema/SchemaSpec.scala:19 

Which kinda suggests that it is trying to compare functions (e.g. fromChunk) which is really suspicious, because equality on functions doesn't really make sense and maybe that's why it fails?

@vigoo
Copy link
Contributor

vigoo commented Nov 24, 2023

In case of Schema.Sequence the functions should not participate in the equality check, only the identity field. Anyway I get the same failed tests now, so I will take a look later today!

@vigoo
Copy link
Contributor

vigoo commented Nov 24, 2023

So that schema equality test did not make any sense - I would say it was accidentally working on JVM (and if you check it, it already had "scala 2 only" annotations, and some cases commented out). That's the reason I wrote proper Eq instances (in StructureEquality) before, so I changed the test to use that.

In ValidationSpec some date-time parsing works differently in scala-java-time than on the JVM, I think we can't really do anything with that so ignored those tests on JS.

There are some more failing tests in the JSON and Protobuf modules.

Copy link
Member Author

@sideeffffect sideeffffect Nov 24, 2023

Choose a reason for hiding this comment

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

@vigoo do you think you could give me some examples (or maybe even all those that we know of?) of strings which parse differently from how they should? I could report these to scala-java-time to get it (eventually) fixed upstream 🙏

Copy link
Contributor

Choose a reason for hiding this comment

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

The following ones can be parsed on JVM and fails on JS. They all seem to fail on the AM/PM postfix:

Error parsing time 00:00:00 AM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '00:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 00:00:00 AM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '00:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 01:01:00 AM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '01:01:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 01:01:00 AM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '01:01:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 10:00:00 AM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '10:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 12:00:00 PM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '12:00:00 PM' could not be parsed, unparsed text found at index 9
Error parsing time 12:09:09 PM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '12:09:09 PM' could not be parsed, unparsed text found at index 9
Error parsing time 12:59:59 PM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '12:59:59 PM' could not be parsed, unparsed text found at index 9
Error parsing time 13:09:09 PM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '13:09:09 PM' could not be parsed, unparsed text found at index 9
Error parsing time 20:20:20 PM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '20:20:20 PM' could not be parsed, unparsed text found at index 9
Error parsing time 22:22:22 PM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '22:22:22 PM' could not be parsed, unparsed text found at index 9
Error parsing time 23:59:00 PM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '23:59:00 PM' could not be parsed, unparsed text found at index 9
Error parsing time 23:59:59 PM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '23:59:59 PM' could not be parsed, unparsed text found at index 9
Error parsing time 24:00:00 AM with format HH:mm:ss a: java.time.format.DateTimeParseException: Text '24:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 0:0:0 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '0:0:0 AM' could not be parsed, unparsed text found at index 6
Error parsing time 1:1:1 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '1:1:1 AM' could not be parsed, unparsed text found at index 6
Error parsing time 00:00:00 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '00:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 00:00:00 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '00:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 01:01:00 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '01:01:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 01:01:00 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '01:01:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 10:00:00 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '10:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 12:00:00 PM with format H:m:s a: java.time.format.DateTimeParseException: Text '12:00:00 PM' could not be parsed, unparsed text found at index 9
Error parsing time 12:09:09 PM with format H:m:s a: java.time.format.DateTimeParseException: Text '12:09:09 PM' could not be parsed, unparsed text found at index 9
Error parsing time 12:59:59 PM with format H:m:s a: java.time.format.DateTimeParseException: Text '12:59:59 PM' could not be parsed, unparsed text found at index 9
Error parsing time 13:09:09 PM with format H:m:s a: java.time.format.DateTimeParseException: Text '13:09:09 PM' could not be parsed, unparsed text found at index 9
Error parsing time 20:20:20 PM with format H:m:s a: java.time.format.DateTimeParseException: Text '20:20:20 PM' could not be parsed, unparsed text found at index 9
Error parsing time 22:22:22 PM with format H:m:s a: java.time.format.DateTimeParseException: Text '22:22:22 PM' could not be parsed, unparsed text found at index 9
Error parsing time 23:59:00 PM with format H:m:s a: java.time.format.DateTimeParseException: Text '23:59:00 PM' could not be parsed, unparsed text found at index 9
Error parsing time 23:59:59 PM with format H:m:s a: java.time.format.DateTimeParseException: Text '23:59:59 PM' could not be parsed, unparsed text found at index 9
Error parsing time 0:00:00 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '0:00:00 AM' could not be parsed, unparsed text found at index 8
Error parsing time 1:11:00 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '1:11:00 AM' could not be parsed, unparsed text found at index 8
Error parsing time 24:00:00 AM with format H:m:s a: java.time.format.DateTimeParseException: Text '24:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 00:00:00 AM with format hh:mm:ss a: java.time.format.DateTimeParseException: Text '00:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 00:00:00 AM with format hh:mm:ss a: java.time.format.DateTimeParseException: Text '00:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 01:01:00 AM with format hh:mm:ss a: java.time.format.DateTimeParseException: Text '01:01:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 01:01:00 AM with format hh:mm:ss a: java.time.format.DateTimeParseException: Text '01:01:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 10:00:00 AM with format hh:mm:ss a: java.time.format.DateTimeParseException: Text '10:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 12:00:00 PM with format hh:mm:ss a: java.time.format.DateTimeParseException: Text '12:00:00 PM' could not be parsed, unparsed text found at index 9
Error parsing time 12:09:09 PM with format hh:mm:ss a: java.time.format.DateTimeParseException: Text '12:09:09 PM' could not be parsed, unparsed text found at index 9
Error parsing time 12:59:59 PM with format hh:mm:ss a: java.time.format.DateTimeParseException: Text '12:59:59 PM' could not be parsed, unparsed text found at index 9
Error parsing time 0:0:0 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '0:0:0 AM' could not be parsed, unparsed text found at index 6
Error parsing time 1:1:1 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '1:1:1 AM' could not be parsed, unparsed text found at index 6
Error parsing time 00:00:00 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '00:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 00:00:00 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '00:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 01:01:00 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '01:01:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 01:01:00 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '01:01:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 10:00:00 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '10:00:00 AM' could not be parsed, unparsed text found at index 9
Error parsing time 12:00:00 PM with format h:m:s a: java.time.format.DateTimeParseException: Text '12:00:00 PM' could not be parsed, unparsed text found at index 9
Error parsing time 12:09:09 PM with format h:m:s a: java.time.format.DateTimeParseException: Text '12:09:09 PM' could not be parsed, unparsed text found at index 9
Error parsing time 12:59:59 PM with format h:m:s a: java.time.format.DateTimeParseException: Text '12:59:59 PM' could not be parsed, unparsed text found at index 9
Error parsing time 0:00:00 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '0:00:00 AM' could not be parsed, unparsed text found at index 8
Error parsing time 1:11:00 AM with format h:m:s a: java.time.format.DateTimeParseException: Text '1:11:00 AM' could not be parsed, unparsed text found at index 8
Error parsing time 00:00:00 000000000 AM with format HH:mm:ss SSSSSSSSS a: java.time.format.DateTimeParseException: Text '00:00:00 000000000 AM' could not be parsed, unparsed text found at index 19
Error parsing time 01:01:00 000000001 AM with format HH:mm:ss SSSSSSSSS a: java.time.format.DateTimeParseException: Text '01:01:00 000000001 AM' could not be parsed, unparsed text found at index 19
Error parsing time 13:09:09 100000000 PM with format HH:mm:ss SSSSSSSSS a: java.time.format.DateTimeParseException: Text '13:09:09 100000000 PM' could not be parsed, unparsed text found at index 19
Error parsing time 20:20:20 123456789 PM with format HH:mm:ss SSSSSSSSS a: java.time.format.DateTimeParseException: Text '20:20:20 123456789 PM' could not be parsed, unparsed text found at index 19
Error parsing time 23:59:00 222222222 PM with format HH:mm:ss SSSSSSSSS a: java.time.format.DateTimeParseException: Text '23:59:00 222222222 PM' could not be parsed, unparsed text found at index 19
Error parsing time 24:00:00 000000000 AM with format HH:mm:ss SSSSSSSSS a: java.time.format.DateTimeParseException: Text '24:00:00 000000000 AM' could not be parsed, unparsed text found at index 19

The following "minute strings" should fail (as they are not a number between 0-59) but the JS version parses them successfully:

Time 60 with format mm should be invalid
Time 90 with format mm should be invalid
Time -1 with format m should be invalid
Time 60 with format m should be invalid
Time 90 with format m should be invalid
Time 0505 with format m should be invalid
Time 123 with format m should be invalid
Time 111 with format m should be invalid

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool, so let's just ignore these testcases until cquiroz/scala-java-time#486 is resolved 👍

@sideeffffect
Copy link
Member Author

But there are also failed tests like this

+ ProtobufCodec Spec
  + Should correctly encode
    + first
    + second
    - integers
      ✗ "08FFFFFF9601" was not equal to "089601"
      e did not satisfy equalTo("089601")
      e = "08FFFFFF9601"
      at /home/runner/work/zio-schema/zio-schema/zio-schema-protobuf/shared/src/test/scala-2/zio/schema/codec/ProtobufCodecSpec.scala:28 
      ✗ "08FFFFFF9601" was not equal to "089601"
      e2 did not satisfy equalTo("089601")
      e2 = "08FFFFFF9601"
      at /home/runner/work/zio-schema/zio-schema/zio-schema-protobuf/shared/src/test/scala-2/zio/schema/codec/ProtobufCodecSpec.scala:28 

which would suggest there is indeed something broken about the codec when run under Scala.js. Somebody familiar with the coded will need to fix this (or generalize this so that it work both for JVM and JS).

@987Nabil
Copy link
Contributor

closing in favour of #638

@987Nabil 987Nabil closed this Jan 15, 2024
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.

3 participants