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

[Feature Request] Add support for other mocking frameworks #99

Closed
SimonMarquis opened this issue Jun 21, 2023 · 5 comments
Closed

[Feature Request] Add support for other mocking frameworks #99

SimonMarquis opened this issue Jun 21, 2023 · 5 comments

Comments

@SimonMarquis
Copy link
Contributor

Would you consider adding support for another popular mocking framework?
We currently have developped our own ad-hoc Lint detector to report wrong usages of mocks using the Mockk.io library.
But I think we could benefit from running this detection only once our codebase.

It could be done either with a lint config parameter, or maybe simpler by adding the relevant classes/methods in these variables:

val MOCK_ANNOTATIONS = setOf("org.mockito.Mock", "org.mockito.Spy")
val MOCK_CLASSES =
setOf(
"org.mockito.Mockito",
"slack.test.mockito.MockitoHelpers",
"slack.test.mockito.MockitoHelpersKt"
)
val MOCK_METHODS = setOf("mock", "spy")

@SimonMarquis
Copy link
Contributor Author

For reference, I tried to build of version of our internal Detector over here:

@ZacSweers
Copy link
Collaborator

I'd be open to making these mock factories configurable, similar to what we do in compose lints for view model injection - https://github.com/slackhq/compose-lints/blob/main/compose-lint-checks/src/main/java/slack/lint/compose/ViewModelInjectionDetector.kt#L25-L31

@SimonMarquis
Copy link
Contributor Author

@ZacSweers I started a draft PR #126 and for now I extracted the previous values as defaults.
I'm not sure what your thoughts are about this (empty defaults, vs existing mockito ones).

Also, regarding the potential tests to be added, what would make more sense here?

  • Duplicate each corresponding slack-lint-checks/src/test/java/slack/lint/mocking/* tests
  • Parameterizing the test suites with JUnit's Parameterized runner
  • Or adding a few tests to cover most cases in a separate test class

@ZacSweers
Copy link
Collaborator

Keeping the defaults sounds good. I think we may also want to unify classes and methods together, i.e. "org.mockito.Mockito.mock" rather than split them.

For tests, keeping the existing defaults while added some basic extra tests to test that custom mocking factories are working I think should be enough.

@ZacSweers
Copy link
Collaborator

Fixed in https://github.com/slackhq/slack-lints/pull/126

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

No branches or pull requests

2 participants