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

remove parallel-test #922

Merged
merged 6 commits into from Aug 15, 2019

Conversation

@nsinkov
Copy link
Collaborator

commented Aug 14, 2019

Changes proposed in this PR

  • remove parallel-test

Why are we making these changes?

parallel-test hangs, replace with eftest

@nsinkov nsinkov requested a review from shamsimam Aug 14, 2019

@shamsimam
Copy link
Contributor

left a comment

How are we configuring the parallelism in eftest? For parallel-test we were using LEIN_TEST_THREADS.

@nsinkov

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 14, 2019

@shamsimam updated

:eftest {:report clojure.test/report}
:eftest {:report clojure.test/report
:thread-count (fn []
(or (some-> (System/getenv "LEIN_TEST_THREADS") Long/valueOf)

This comment has been minimized.

Copy link
@shamsimam

shamsimam Aug 14, 2019

Contributor

thread-count eventually calls Executors/newFixedThreadPool which expect an int argument.

@nsinkov

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 14, 2019

updated

nsinkov added 2 commits Aug 14, 2019
@@ -111,11 +111,12 @@
[slingshot "0.12.2"]
[try-let "1.3.1"
:exclusions [org.clojure/clojure]]]
:eftest {:report clojure.test/report}
:eftest {:report clojure.test/report
:thread-count (or (some-> (System/getenv "LEIN_TEST_THREADS") Integer/valueOf)

This comment has been minimized.

Copy link
@shamsimam

shamsimam Aug 15, 2019

Contributor

Should we do parseInt instead of valueOf?
availableProcessors also returns an int.

:resource-paths ["resources"]
:main waiter.main
:plugins [[com.holychao/parallel-test "0.3.2"]
[lein-exec "0.3.7"]
:plugins [[lein-exec "0.3.7"]
[test2junit "1.2.2"]
[lein-eftest "0.5.8"]]

This comment has been minimized.

Copy link
@shamsimam

shamsimam Aug 15, 2019

Contributor

Nitpick: lexicographic order

@shamsimam shamsimam merged commit 7eafae2 into twosigma:master Aug 15, 2019

0 of 2 checks passed

Mergeable Mergeable run returned Status ***FAIL***
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.