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

Move Android Studio related code out of GradleScenarioInvoker and rel… #374

Merged
merged 3 commits into from
Dec 7, 2021

Conversation

asodja
Copy link
Member

@asodja asodja commented Nov 22, 2021

…ated classes

So yeah, I did not introduce generics to BuildAction at the moment since this can complicate things. So right now I just wrap Gradle invoker logic and enhance samples, here is some casting involved that might not be that nice (in StudioSample class), but it makes a solution simpler.

Fixes: #371

@asodja asodja requested review from lptr and removed request for lptr November 22, 2021 19:18
@asodja asodja added @execution internal Internal change labels Nov 22, 2021
@asodja asodja force-pushed the asodja/after-studio-refactoring branch from 9b641db to 343f2a6 Compare November 22, 2021 19:22
@asodja asodja requested a review from lptr November 22, 2021 19:27
@asodja asodja self-assigned this Nov 22, 2021
@asodja asodja force-pushed the asodja/after-studio-refactoring branch from 343f2a6 to 659a68a Compare November 22, 2021 19:35
@asodja asodja force-pushed the asodja/after-studio-refactoring branch from 659a68a to 4777180 Compare November 23, 2021 12:03
GRADLE_EXECUTION_TIME("Gradle execution time") {
@Override
public Duration extractFrom(BuildInvocationResult result) {
StudioBuildActionResult studioResult = (StudioBuildActionResult) result.getActionResult();
Copy link
Member

Choose a reason for hiding this comment

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

We don't want to introduce a StudioBuildInvocationResult to avoid this cast?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would be ideal, but it requires some changes, I will check what I can do.

Copy link
Member Author

@asodja asodja Dec 1, 2021

Choose a reason for hiding this comment

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

What I did: I created a StudioBuildInvocationResult, and I do a cast there. It's not totally perfect, but I think that right now that is the simplest and cleanest solution.

Otherwise, we would have to inject (or override some method that will return) a custom BuildUnderTestInvoker into GradleScenarioInvoker and also create a new InvokeAndMeasureAction that would return StudioBuildInvocationResult. Additionally to that, BuildAction would have to return generic result from run method and that would require passing generics everywhere and it will get a bit messy.

I would try to avoid that. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Alright, one step at a time.

@asodja asodja force-pushed the asodja/after-studio-refactoring branch from 7528009 to b8f6c61 Compare December 1, 2021 21:23
@asodja asodja force-pushed the asodja/after-studio-refactoring branch from b8f6c61 to 71a64c3 Compare December 1, 2021 21:39
@lptr lptr self-requested a review December 2, 2021 08:29
Copy link
Member

@lptr lptr left a comment

Choose a reason for hiding this comment

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

LGTM.

@asodja asodja merged commit df9ede4 into master Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Internal change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move IDE related code out of GradleScenarioInvoker
2 participants