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

No way to run the tests since jbuilder? #32

Closed
rixed opened this issue Feb 19, 2019 · 11 comments · Fixed by #44
Closed

No way to run the tests since jbuilder? #32

rixed opened this issue Feb 19, 2019 · 11 comments · Fixed by #44

Comments

@rixed
Copy link
Contributor

rixed commented Feb 19, 2019

I can't find how to run the tests since the build system is jbuilder.
Could this be documented in the README maybe?

Note: I suspect the comparison function is wrong for large int128 and would like to fix it

@ghost
Copy link

ghost commented Nov 28, 2019

I'm not the maintainer and can't say if it should be in the README, but have you tried jbuilder runtest? It should change to dune runtest soon (#35).

@rixed
Copy link
Contributor Author

rixed commented Nov 28, 2019

Thank you. Will try to go back to this comparison function in a few weeks.

@rixed
Copy link
Contributor Author

rixed commented Dec 1, 2019

Actually jbuilder runtest does not do anything it seems, thus the issue I guess.

@ghost
Copy link

ghost commented Dec 1, 2019

The project was migrated to jbuilder in 2ed2e97, and even though the old build files seem to reference testing, I'm not sure where they are and how to integrate them with dune. Are there any tests in the repo? @rgrinberg, thoughts?

@rgrinberg
Copy link
Collaborator

Yeah that seems like an omission on my part.

@rgrinberg
Copy link
Collaborator

Is it possible to submit #35, and perhaps have you work on the tests? :)

Sorry, I'm a bit swamped at the moment. I could certainly help you out if you have any questions.

@ghost
Copy link

ghost commented Dec 2, 2019

I don't mind extending the branch or opening a follow-up pull request, in case you can (quickly) merge #35 first. To that end, how do we integrate the existing tests with Dune? Actually, are there any tests in the repo, and did I fail to uncover them? If there are none, I'm afraid I can't commit to writing them first, since I don't use stdint directly and only did the dune migration to avoid jbuilder in opam-repository.

That said, if there are no tests, I don't mind adding a minimum/trivial test, in order to make runtest do something. That way, someone else can implement a test suite with dune integration already taken care of.

@rgrinberg
Copy link
Collaborator

rgrinberg commented Dec 2, 2019 via email

@ghost
Copy link

ghost commented Dec 3, 2019

Right, spec/* are meant to be executed with https://github.com/andrenth/ospec. Since OSpec's opam entry says <4.06.0, it looks like making the tests work and integrating them with dune is more work than should be bundled into #35.

@rgrinberg
Copy link
Collaborator

@rixed would you like to revive the tests perahps? it should be sufficient to functorize common.ml and instantiate for the different integer types. Ospec can be dropped in favor of alcotest or ppx_inline_test.

@rixed
Copy link
Contributor Author

rixed commented Dec 3, 2019

@rgrinberg I was just looking at it, but ospec uses camlp4 so that's more work that I'm able to spend right now. I might have more time in a few weeks though.

rixed added a commit to rixed/ocaml-stdint that referenced this issue Feb 7, 2020
Those are simpler (only relying on qcheck rather than the unmaintained
ospec (https://github.com/andrenth/ospec)).
Also believed to be more exhaustive.
Also added a few more tests.

Not all tests currently pass (11/282 are failing).
I believe most of the failures would have failed earlier, would the
former tests run on all types.

Some of the failure are because of the tests being wrong, some look like
genuine errors.

Having no time to learn a specific build system for every programming
language, I've added a small Makefile that will build and run the tests.
I'm confident a dune zealot will quickly and properly merge this into
the dune configuration. :)

Closes andrenth#32
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 a pull request may close this issue.

2 participants