-
Notifications
You must be signed in to change notification settings - Fork 352
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
Add konsist tests #5214
Add konsist tests #5214
Conversation
7c6bfee
to
4d84d22
Compare
4d84d22
to
d7382e8
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.
Reviewed 32 of 33 files at r1, 42 of 43 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
228ded3
to
14040be
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.
Reviewed 33 of 43 files at r2, 2 of 3 files at r4, 35 of 35 files at r5, all commit messages.
Reviewable status: 75 of 76 files reviewed, 3 unresolved discussions (waiting on @Rawa)
android/test/arch/src/test/kotlin/net/mullvad/mullvadvpn/test/arch/ArchitectureTests.kt
line 11 at r5 (raw file):
@Test fun `domain layer depends on nothing`() {
I believe we should to refer to these as model(s) (or perhaps combine it somehow with "domain") until we've settled on a architectural long-term plan
Code quote:
domain
android/test/arch/src/test/kotlin/net/mullvad/mullvadvpn/test/arch/GeneralTests.kt
line 12 at r5 (raw file):
@Test fun `package name must match file path`() { Konsist.scopeFromProject().packages.assert { it.hasMatchingPath }
Replace with the existing projectScope()
extension. The same applies to the other test functions in this class
Code quote:
Konsist.scopeFromProject()
android/test/arch/src/test/kotlin/net/mullvad/mullvadvpn/test/arch/extensions/KonsistExtensions.kt
line 1 at r5 (raw file):
package net.mullvad.mullvadvpn.test.arch.extensions
I'm not sure whether this file is necessary or not. I believe it would be easiest to understand the code if we add all/most of the scope calls or none of them here to avoid a mix. Perhaps we can discuss this further outside of GH/Reviewable.
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.
Reviewed 3 of 3 files at r4, 15 of 35 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Rawa)
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.
Reviewable status: 63 of 76 files reviewed, 3 unresolved discussions (waiting on @albin-mullvad, @Pururun, and @sabercodic)
android/test/arch/src/test/kotlin/net/mullvad/mullvadvpn/test/arch/ArchitectureTests.kt
line 11 at r5 (raw file):
Previously, albin-mullvad wrote…
I believe we should to refer to these as model(s) (or perhaps combine it somehow with "domain") until we've settled on a architectural long-term plan
Good point
android/test/arch/src/test/kotlin/net/mullvad/mullvadvpn/test/arch/GeneralTests.kt
line 12 at r5 (raw file):
Previously, albin-mullvad wrote…
Replace with the existing
projectScope()
extension. The same applies to the other test functions in this class
Aligned all tests
android/test/arch/src/test/kotlin/net/mullvad/mullvadvpn/test/arch/extensions/KonsistExtensions.kt
line 1 at r5 (raw file):
Previously, albin-mullvad wrote…
I'm not sure whether this file is necessary or not. I believe it would be easiest to understand the code if we add all/most of the scope calls or none of them here to avoid a mix. Perhaps we can discuss this further outside of GH/Reviewable.
Done.
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.
Reviewed 2 of 43 files at r2, 1 of 3 files at r4, 13 of 13 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
b744cc3
to
c7279b8
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.
Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
c7279b8
to
3f6a24d
Compare
Checks that the tests are prefixed with "ensure "
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.
Dismissed @albin-mullvad from a discussion.
Reviewable status: 71 of 77 files reviewed, all discussions resolved (waiting on @albin-mullvad, @Pururun, and @sabercodic)
android/test/arch/src/test/kotlin/net/mullvad/mullvadvpn/test/arch/GeneralTests.kt
line 12 at r5 (raw file):
Previously, Rawa (David Göransson) wrote…
Aligned all tests
Done.
This PR adds a bunch of konsist test to ensure we adhere to nice code standards.
This change is