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 anagram unit test for Base v0.12.0 #301

Merged
merged 1 commit into from
Apr 8, 2019
Merged

Fix anagram unit test for Base v0.12.0 #301

merged 1 commit into from
Apr 8, 2019

Conversation

citizen428
Copy link
Contributor

Today I spent way too much time looking for an error in my own code before I realized it actually came from the unit test 🤦‍♂️

Here's the error:

File "test.ml", line 7, characters 47-59:
Error: The function applied to this argument has type
         ('a -> 'a -> bool) -> 'a list -> 'a list -> bool

Coming from the following line:

assert_equal exp got ~cmp:(List.equal ~equal:String.equal) ~printer

This can be fixed either by using

assert_equal exp got ~cmp:(List.equal String.equal) ~printer

or by just leaving off ~cmp: completely:

assert_equal exp got ~printer

Looking at Base's changelog, the following seems to be the culprit:

  • Changed the signature of equal from 'a t -> 'a t -> equal:('a -> 'a -> bool) -> bool to ('a -> 'a -> bool) -> 'a t -> 'a t -> bool.

@sshine
Copy link
Contributor

sshine commented Apr 8, 2019

OK, excellent. You don't need to force push, since GitHub lets me squash merge.

@sshine sshine merged commit da5e341 into exercism:master Apr 8, 2019
@citizen428
Copy link
Contributor Author

You don't need to force push, since GitHub lets me squash merge.

I know, but I've seen too many people forget about it and since I'm gonna delete my fork anyway I generally err on the side of caution. 😃

sshine pushed a commit that referenced this pull request Jun 21, 2019
Use the `ocaml/opam2` Docker image for CI.

Explicitly use OCaml 4.07.0 for now.

Pin base to v0.11.1 to avoid breaking changes as documented in #301.

This fixes #308.
marionebl pushed a commit to marionebl/ocaml that referenced this pull request Jun 24, 2019
In Ocaml Base v0.12.0 [1], the following breaking change causes Anagram's unit 
tests to fail:

- Changed the signature of `equal` from `'a t -> 'a t -> equal:('a -> 'a ->
  bool) -> bool` to `('a -> 'a -> bool) -> 'a t -> 'a t -> bool`.

We choose to fix this in a way that is compatible with both v0.11 and v0.12 by
leaving off `~cmp:` completely, relying on implicit, polymorphic compare.

[1]: https://github.com/janestreet/base/blob/master/CHANGES.md
marionebl added a commit to marionebl/ocaml that referenced this pull request Jun 24, 2019
Use the `ocaml/opam2` Docker image for CI.

Explicitly use OCaml 4.07.0 for now.

Pin base to v0.11.1 to avoid breaking changes as documented in exercism#301.

This fixes exercism#308.
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