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

fix test-laplace-petsc3d #1985

Merged
merged 2 commits into from
Mar 24, 2020
Merged

fix test-laplace-petsc3d #1985

merged 2 commits into from
Mar 24, 2020

Conversation

dschwoerer
Copy link
Contributor

fix for part of #1981

@dschwoerer dschwoerer added bugfix small-change Changes less than 100 lines - should be quick to review labels Mar 23, 2020
@dschwoerer
Copy link
Contributor Author

openmpi seems to also include . in PATH, however mpich doesn't.

johnomotani
johnomotani previously approved these changes Mar 23, 2020
@ZedThree
Copy link
Member

Might also be an idea to just remove the try ... except? It's probably more useful to have the actual output printed and not do the rest of the tests, especially on Travis

@johnomotani
Copy link
Contributor

Might also be an idea to just remove the try ... except? It's probably more useful to have the actual output printed and not do the rest of the tests, especially on Travis

I agree. The try...except was useful for developing/debugging the solver, but now that the tests do all pass, and should continue to, it would be better to fail immediately if one of them fails.

@dschwoerer
Copy link
Contributor Author

Updated.

In that case test-communications could also be updated to fail early and not print the errors at the end, without context

@johnomotani
Copy link
Contributor

In that case test-communications could also be updated to fail early and not print the errors at the end, without context

Done in #1983.

@ZedThree ZedThree merged commit 19ed331 into next Mar 24, 2020
@ZedThree ZedThree deleted the fix-test-laplace-petsc3d branch March 24, 2020 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix small-change Changes less than 100 lines - should be quick to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants