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

ocamltest: fix `bsd` and `not-bsd` built-in actions #2223

Merged
merged 3 commits into from Feb 7, 2019

Conversation

Projects
None yet
3 participants
@damiendoligez
Copy link
Member

commented Jan 18, 2019

On our CI, the openBSD machine accumulates leftover processes from the lib-systhreads/testfork.ml test, which is not supposed to run on BSD since it has the condition not-bsd.

Instead of testing only for bsd_elf, ocamltest should test for all 4 possible BSD values.

@damiendoligez damiendoligez requested a review from shindere Jan 18, 2019

@dra27
Copy link
Contributor

left a comment

Is this why the OpenBSD worker has been taking so long?

"on a BSD system"
"not on a BSD system")

let not_bsd = make
"not-bsd"
(Actions_helpers.pass_or_skip (Ocamltest_config.system <> bsd_system)
(Actions_helpers.pass_or_skip (not (is_bsd_system Ocamltest_config.system))

This comment has been minimized.

Copy link
@dra27

dra27 Jan 18, 2019

Contributor

Probably not the time now, but having had to add several “not” predicates, we really should refactor these to:

let bsd, not_bsd = make_predicate
  "bsd"
  (Actions_helpers.pass_or_skip (is_bsd_system Ocamltest_config.system)
    "on a BSD system")
let bsd_system = "bsd_elf"
let is_bsd_system s =
match s with
| "bsd_elf" | "netbsd" | "freebsd" | "openbsd" -> true

This comment has been minimized.

Copy link
@dra27

dra27 Jan 18, 2019

Contributor

Would it be sound just to match anything containing "bsd"?

This comment has been minimized.

Copy link
@damiendoligez

damiendoligez Feb 7, 2019

Author Member

That was my first idea, but OCaml's pattern-matching doesn't support regexps or even substring matching, so I decided to keep it simple.

@shindere

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2019

@shindere

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2019

@damiendoligez

This comment has been minimized.

Copy link
Member Author

commented Feb 7, 2019

IME, everything starts working again when I manually kill the leftover processes on the OpenBSD worker...

@damiendoligez damiendoligez merged commit 6015d60 into ocaml:trunk Feb 7, 2019

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.