-
Notifications
You must be signed in to change notification settings - Fork 5
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
Split tests #53
Split tests #53
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #53 +/- ##
==========================================
+ Coverage 98.68% 98.80% +0.11%
==========================================
Files 11 11
Lines 834 834
==========================================
+ Hits 823 824 +1
+ Misses 11 10 -1 ☔ View full report in Codecov by Sentry. |
934df20
to
6fa2e48
Compare
…ok_for_disco into cets_disco_SUITE
336eed6
to
c19a3c4
Compare
ad524e5
to
e3ad79c
Compare
c40bf75
to
07839a6
Compare
c98d019
to
4f5b0de
Compare
cover logic is broken in Erlang 26. So, peer:stop/1 should not be called
4f5b0de
to
b9908fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Moves the suites to different suites in a sensible manner, and introduces relevant helpers, it's a great effort!
I've left some minor comments, mostly about unused import statements, and maybe moving some helper functions around.
I didn't really look into the test logic, but from what I see the tests are mostly moved around and the new changes make sense to me 👍
State#{other_servers := Servers} | ||
end). | ||
|
||
set_join_ref(Pid, JoinRef) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe if one testcase is moved from cets_SUITE
to cets_join_SUITE
this could be moved to cets_join_SUITE
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like suites contain test cases, helpers could go into a separate file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I don't mind that. 👍
I just worry a bit with a general test helper, it may contain everything but the kitchen sink after some time, but it also makes sense to keep the suites clean.
@@ -0,0 +1,37 @@ | |||
-module(cets_test_helper). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must admit I'm not a fan of a general helper, especially if there are specific helpers, but I'm not sure how to fix this.
I've left some comments which may make sense if we reshuffle some testcases, but I also understand if we want to move helper functions out of the *_SUITE
files altogether - then this module is needed as it is I believe.
This PR addresses MIM-2187.
Separate cets_SUITE into parts
cets_SUITE is getting big, and big files are hard to understand/maintain.