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

Sanitise README and tests to help other contributors #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Sanitise README and tests to help other contributors #13

wants to merge 1 commit into from

Conversation

ashleyfrieze
Copy link
Contributor

  • fix minor typos
  • allow tests to pass when run in IntelliJ (avoid unrunnable test classes)
  • enhance info about building and running tests

- fix minor typos
- allow tests to pass when run in IntelliJ (avoid unrunnable test classes)
- enhance info about building and running tests
@@ -7,7 +7,7 @@
import static com.github.stefanbirkner.fishbowl.Fishbowl.ignoreException;
import static org.assertj.core.api.Assertions.assertThat;

class RestoreSystemErrChecks {
abstract class RestoreSystemErrChecks {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a test base class and JUnit5 cannot run it. If marked abstract the IDE won't try to run it either. Obviously, its name prevents it being run by surefire, but this covers both angles.

@stefanbirkner
Copy link
Owner

stefanbirkner commented Nov 9, 2020

Thanks for your work.

The changes in README.md have been merged: 8903a1d.

I still think about whether to merge the change with abstract. Not sure about this. I want to first try out something with JUnit Jupiter.

I will not merge the .gitignore changes (see #7 for details).

@ashleyfrieze
Copy link
Contributor Author

Thanks for your work.

The changes in README.md have been merged: 8903a1d.

I still think about whether to merge the change with abstract. Not sure about this. I want to first try out something with JUnit Jupiter.

I will not merge the .gitignore changes (see #7 for details).

If a test base class cannot be run directly, then it should be abstract that's been true for JUnit4 and 5. It's another nicety to people running tests in their IDEs.

As we discussed in #9, I'm forking out this codebase to create something which allows many more patterns of invocation. As I do so, I'll try to contribute back any fixes for glitches that I find in the core code. It's up to you whether you include them, but it's only fair to pay you back with any improvements that translate into your paradigm.

@stefanbirkner stefanbirkner force-pushed the master branch 3 times, most recently from d3ce0eb to 5e206a7 Compare August 14, 2021 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants