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

Simplify Flacoco results #91

Merged
merged 3 commits into from
Aug 25, 2021

Conversation

andre15silva
Copy link
Member

Closes #88

Flacoco would work like this:

  • Only one entrypoint, the run method of Flacoco.
  • The result is always the same object type FlacocoResult. This class encapsulates the information we have now, and possibly more information in the future.
  • The Spoon mode is now available through a config option in FlacocoConfig. Set the option there, and the results will be available in FlacocoResult along side the default.
  • The default result's key if now a Location object, which for now contains a class name and a line number.
  • The class name in the default key is now separated by . as usual, not by / as we have been considering.

Any suggestion? This should be the definitive output format of flacoco and where we will build on top of.

@martinezmatias
Copy link
Collaborator

Hi @andre15silva

Flacoco would work like this:

Only one entrypoint, the run method of Flacoco.

Nice. Easier than the previous version where we had two methods.

The result is always the same object type FlacocoResult. This class encapsulates the information we have now, and possibly more information in the future.

Perfect.

The Spoon mode is now available through a config option in FlacocoConfig. Set the option there, and the results will be available in FlacocoResult along side the default.

Just one idea: should FlacocoResult throw an exception when the user calls getSpoonSuspiciousnessMap but the computeSpoonResults from FlacocoConfig is false?

The default result's key if now a Location object, which for now contains a class name and a line number.

Perfect. For the moment is fine. I wonder if it would necessary to relate Locations with CtStatements when computeSpoonResults is true (i.e., Locations has a reference to a CtStatement/CtElement)

The class name in the default key is now separated by . as usual, not by / as we have been considering.

Perfect.

Thanks!
Matias

@andre15silva
Copy link
Member Author

Just one idea: should FlacocoResult throw an exception when the user calls getSpoonSuspiciousnessMap but the computeSpoonResults from FlacocoConfig is false?

I think that's maybe not such a good idea, because FlacocoConfig is a singleton and there might be several instances of FlacocoResult at the same time. As it is, the user has to look for the null, but I'll make that very explicit in the documentation.

I wonder if it would necessary to relate Locations with CtStatements when computeSpoonResults is true (i.e., Locations has a reference to a CtStatement/CtElement)

We could have an additional mapping, it is really simple to add.

@martinezmatias
Copy link
Collaborator

I think that's maybe not such a good idea, because FlacocoConfig is a singleton and there might be several instances of FlacocoResult at the same time.

Good point.

because FlacocoConfig is a singleton and there might be several instances of FlacocoResult at the same time.

I wonder if that singleton architecture could impact on having several instances of FlacocoResult at the same time (e.g., when we launch flacoco in two different bugs).
Maybe we can discuss in a new issue.

As it is, the user has to look for the null, but I'll make that very explicit in the documentation.

Fine. Let's put that in the doc, and even in the JavaDoc.

We could have an additional mapping, it is really simple to add.

If it's simple, I would propose to add it. It could be useful in the future.

Thanks
Matias

@andre15silva
Copy link
Member Author

I wonder if that singleton architecture could impact on having several instances of FlacocoResult at the same time (e.g., when we launch flacoco in two different bugs).
Maybe we can discuss in a new issue.

I don't see a case where that would happen, since all the objects inside of FlacocoResult are their own instance. Am I missing something?

and even in the JavaDoc.

Done!

If it's simple, I would propose to add it. It could be useful in the future.

Done!

@martinezmatias
Copy link
Collaborator

Hi @andre15silva

and even in the JavaDoc.

Done!

If it's simple, I would propose to add it. It could be useful in the future.

Done!

Thanks!

since all the objects inside of FlacocoResult are their own instance. Am I missing something?

I agree that all the objects inside of FlacocoResult are their own instance, that is not the problem. However, I think all runners share the same FlacocoConfiguration as it's a singleton, even each of them could be configured differently. I open an issue for discussing that: #92

@martinezmatias
Copy link
Collaborator

nice @andre15silva , thanks!

@martinezmatias martinezmatias merged commit 982a803 into ASSERT-KTH:master Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Simplify output
2 participants