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

Support multi module tests #137

Merged
merged 6 commits into from
Nov 3, 2020
Merged

Conversation

yigit
Copy link
Collaborator

@yigit yigit commented Nov 1, 2020

This PR adds support for multi-module tests.
Unfortunately, the current base class we use for tests does not support it. Instead, I've changed base ksp test to directly extend KotlinBaseTest and added a test file collector that supports modules.

Right now, we only run KSP in the last module. It can be changed later if necessary.

This does not have any impact on existing tests as we create a default module if none provided.

yigit added 3 commits October 31, 2020 16:49
This PR changes ksp abstract test to support modules.

I'm using a directive (MODULE:) that already exists in
kotlin codebase but rest of module handling is implemented
manually where each sub-module is compiled as is (w/o ksp)
and then final module is compiled with KSP.

Test: MultiModuleTestProcessor
@yigit
Copy link
Collaborator Author

yigit commented Nov 2, 2020

there is a relatively small problem i noticed here where the ResolverImpl do not get the correct module name.
It does not affect for now but eventually we need to pass the right module name to the TypeMapper (we pass default right now) and then it might matter.
I'll figure it out and do a followup CL for that.

I've also changed resolver to pass this name to the type
mapper. It is a bit hacky because kotlin passes down a special
name which we need to revert back to the original
@yigit
Copy link
Collaborator Author

yigit commented Nov 3, 2020

added a fix for the module name:
9688126

@yigit yigit requested a review from neetopia November 3, 2020 17:58
@neetopia neetopia merged commit c844f2a into google:master Nov 3, 2020
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