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

Optimize method and hybrid starts by removing unecessary affected test classes computation #110

Conversation

mmk-1
Copy link

@mmk-1 mmk-1 commented Oct 28, 2023

  • I've removed the parts that computed affectedTestClasses for optimization.
  • nonAffectedMethods variable was not used anyways in the code so I removed that too
  • As an outcome of removing affectedTestClasses computations, methodToTestClasses mapping wasn't needed in HybridMojo.java either.

@owolabileg
Copy link
Collaborator

@mmk-1 let's think carefully about these changes, before merging them.

I've removed the parts that computed affectedTestClasses for optimization.

IIUC, these affectedTestClasses maps method-level changes to the class level. Is that right? If so, we'd need something like that for experiments. So, instead of deleting, why not make it an option that the user can enable or disable?

nonAffectedMethods variable was not used anyways in the code so I removed that too

It seems that you would need this variable when you later implement all the variants (e.g., to avoid instrumenting nonAffected methods. Does that make sense?

As an outcome of removing affectedTestClasses computations, methodToTestClasses mapping wasn't needed in HybridMojo.java either.

Please see my comment above: should we make it an option instead of outright deleting it?

@mmk-1
Copy link
Author

mmk-1 commented Oct 29, 2023

Yes you're right. I'll make changes to introduce a parameter to turn this feature on/off

@mmk-1 mmk-1 force-pushed the optimize-remove-affectedtests branch from 5d48df2 to e6737a5 Compare October 31, 2023 08:51
@mmk-1
Copy link
Author

mmk-1 commented Oct 31, 2023

@owolabileg I have added new parameters based on your reviews to make Affected Test Class computation optional. Please review. Thanks!

@owolabileg
Copy link
Collaborator

This all looks good to me, and I think you can merge.

@owolabileg owolabileg merged commit b563ddf into TestingResearchIllinois:mixed-granularity Nov 7, 2023
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