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

ocamlPackages: add crowbar, enable crowbar tests #88475

Open
wants to merge 3 commits into
base: master
from

Conversation

@sternenseemann
Copy link
Member

sternenseemann commented May 21, 2020

Motivation for this change

Add crowbar, an afl based test framework, which some test suites in ocamlPackages depend on (index and eqaf as far as I know which have their test suites enabled in this PR).

One issue this PR currently has is that it probably doesn't make sense to run the crowbar tests on a compiler which does not support afl. We could detect afl and enable/disable the tests like this (although its not nice):

{
  doCheck = lib.hasInfix "afl" ocaml.name;
}

However, this has another issue: The affected packages have a normal test suite as well which doesn't require crowbar. Selectively activating and deactivating parts of the test suite could become rather cumbersome and I think it is not really that big of an issue since crowbar runs and compiles without any errors even when afl is not enabled.

Note that this PR also includes the eqaf update from #88467.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@sternenseemann sternenseemann force-pushed the sternenseemann:crowbar-0.2 branch 2 times, most recently from 1129f5b to 9ffddee May 21, 2020
@sternenseemann sternenseemann force-pushed the sternenseemann:crowbar-0.2 branch from 9ffddee to 921713a May 21, 2020
@sternenseemann sternenseemann force-pushed the sternenseemann:crowbar-0.2 branch from 921713a to 3bb62e9 May 22, 2020
@sternenseemann
Copy link
Member Author

sternenseemann commented May 22, 2020

@GrahamcOfBorg build jackline ocamlPackages.irmin-unix

@sternenseemann
Copy link
Member Author

sternenseemann commented May 22, 2020

The eqaf test just failed for me, will investigate.

@sternenseemann
Copy link
Member Author

sternenseemann commented May 22, 2020

I think it failed on me once in nixpkgs-review, but I can't reproduce it anymore, maybe someone else can run the tests as well (eqaf tests take forever unfortunately).

@vbgl
Copy link
Contributor

vbgl commented May 24, 2020

@GrahamcOfBorg build ocamlPackages.afl-persistent

@vbgl
Copy link
Contributor

vbgl commented May 24, 2020

afl-persistent has been merged into master as 0679fb9

@vbgl
Copy link
Contributor

vbgl commented May 29, 2020

@GrahamcOfBorg build ocamlPackages.crowbar

@sternenseemann sternenseemann force-pushed the sternenseemann:crowbar-0.2 branch from 3bb62e9 to 97dad4b Jun 9, 2020
@sternenseemann sternenseemann force-pushed the sternenseemann:crowbar-0.2 branch from 97dad4b to 0c11190 Jun 9, 2020
@sternenseemann
Copy link
Member Author

sternenseemann commented Jun 9, 2020

@GrahamcOfBorg build ocamlPackages.index

@vbgl vbgl dismissed their stale review Jun 9, 2020

afl-persistent has been merged

@vbgl
Copy link
Contributor

vbgl commented Jun 9, 2020

crowbar has been merged into master as 1c7fd15

@vbgl
Copy link
Contributor

vbgl commented Jun 9, 2020

Is it wise to enable tests that “take forever”?

@sternenseemann
Copy link
Member Author

sternenseemann commented Jun 9, 2020

Probably not, since then builds will timeout all the time, although it'd be nice to have eqaf tests since it is kind of a security sensitive library. Hydra has a build timeout right?

Also it'd be really frustrating to use that library with ocaml versions where there are no prebuild binaries since you can't disable the tests easily then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.