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

OTP 26 test failure #17

Closed
ckampfe opened this issue Jun 26, 2023 · 3 comments
Closed

OTP 26 test failure #17

ckampfe opened this issue Jun 26, 2023 · 3 comments

Comments

@ckampfe
Copy link
Contributor

ckampfe commented Jun 26, 2023

On OTP 26 (both Elixir 1.14.5 and 1.15.0) I'm getting this test failure:

  1) test create attribute string from map (SneezeInternalTest)
     test/sneeze_test.exs:186
     Assertion with == failed
     code:  assert Internal.attributes_to_iolist(%{class: "foo", id: "bar"}) == [
              [" ", "class", "=\"", "foo", "\""],
              [" ", "id", "=\"", "bar", "\""]
            ]
     left:  [
              [" ", "id", "=\"", "bar", "\""],
              [" ", "class", "=\"", "foo", "\""]
            ]
     right: [
              [" ", "class", "=\"", "foo", "\""],
              [" ", "id", "=\"", "bar", "\""]
            ]
     stacktrace:
       test/sneeze_test.exs:187: (test)

............................
Finished in 0.06 seconds (0.00s async, 0.06s sync)
30 tests, 1 failure

Randomized with seed 682196

which I believe is due to the (implicit) assumption that the map arg (%{class: "foo", id: "bar"}) will be iterated first by the class key and then the id key.

I believe this assumption which was previously valid is no longer valid because in the OTP team in OTP 26 made an optimization to maps which changed the iteration order of maps from being by the term order of the keys to being undefined:
"The new order is undefined and may change between different invocations of the Erlang VM." (https://www.erlang.org/blog/otp-26-highlights/#changed-ordering-of-atom-keys)

I've previously run into issues like this, and solved them by comparing the collections as sets (MapSet.equal?(s1, s2)) rather than lists, if order is not important. I'm happy to PR this change if you want it or discuss further if you have other ideas.

Thanks again for your efforts on this library.

@JuneKelly
Copy link
Owner

JuneKelly commented Jun 26, 2023

Thanks for opening this! I think it might be safer to sort the entries coming out of the map, so we get a stable ordering going forward. I'll look into this soon.

@JuneKelly
Copy link
Owner

@ckampfe , I've pushed something that I think will fix this in #18

@JuneKelly
Copy link
Owner

This is fixed in 2.0.0 🎉

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

No branches or pull requests

2 participants