Skip to content

ocamlPackages: add crowbar, enable crowbar tests #88475

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

ocamlPackages: add crowbar, enable crowbar tests #88475

wants to merge 2 commits into from
+9 −2

Conversation

sternenseemann
Copy link
Member

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.

Sorry, something went wrong.

@sternenseemann sternenseemann force-pushed the crowbar-0.2 branch 2 times, most recently from 1129f5b to 9ffddee Compare May 21, 2020 01:55
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels May 21, 2020
vbgl
vbgl previously requested changes May 21, 2020
View reviewed changes
@sternenseemann
Copy link
Member Author

@GrahamcOfBorg build jackline ocamlPackages.irmin-unix

@sternenseemann
Copy link
Member Author

The eqaf test just failed for me, will investigate.

@sternenseemann
Copy link
Member Author

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 •
edited
Loading

@GrahamcOfBorg build ocamlPackages.crowbar

@sternenseemann sternenseemann force-pushed the crowbar-0.2 branch 2 times, most recently from 97dad4b to 0c11190 Compare June 9, 2020 15:44
@sternenseemann
Copy link
Member Author

@GrahamcOfBorg build ocamlPackages.index

@vbgl vbgl dismissed their stale review June 9, 2020 15:59

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 •
edited
Loading

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.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Jul 25, 2020
@stale
Copy link

stale bot commented Jan 21, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vbgl vbgl

Assignees
No one assigned
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sternenseemann @vbgl