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

Clean amplifiers implementation #364

Closed
sbihel opened this issue Mar 13, 2018 · 9 comments

Comments

@sbihel
Copy link
Contributor

@sbihel sbihel commented Mar 13, 2018

Inconsistencies within input amplifiers:

  • the amplification counter is sometimes incremented (e.g. in TestMethodCallAdder) and sometimes not (e.g. in StatementAdd); and
  • the map ampTestToParent keeping the links between amplified tests and the parent test is sometimes updated (e.g. in TestMethodCallAdder) and sometimes not (e.g. in StatementAdd).

Inconsistency within amplifiers:

  • the map ampTestToParent is sometimes updated directly (e.g. in MethodsAssertGenerator) and sometimes is updated through a buffer updateAmpTestToParent (e.g. in TestMethodCallAdder).

I propose the following changes:

  • increment the amplification counter in the cloning method, requiring to have 2 cloning methods, one for each amplification category;
  • update the parenting link in the cloning method, always using the buffer;
  • prevent direct modification to ampTestToParent from outside; and
  • rename the interface Amplifier to InputAmplifier.
@danglotb

This comment has been minimized.

Copy link
Member

@danglotb danglotb commented Mar 14, 2018

Hi @sbihel

Thank you very much for reporting issues inside the code of DSpot.

I agree with the proposed changes.

Are you working on them?

@sbihel

This comment has been minimized.

Copy link
Contributor Author

@sbihel sbihel commented Mar 14, 2018

Hi @danglotb,

Are you working on them?

I am planning to 😀 I wanted to make sure I was not going in the wrong direction

@danglotb

This comment has been minimized.

Copy link
Member

@danglotb danglotb commented Mar 14, 2018

Okay! Sorry for the delay! I can't wait to see your progress!

@danglotb

This comment has been minimized.

Copy link
Member

@danglotb danglotb commented Mar 20, 2018

Hi @sbihel

I saw that you closed your PR. I know that I merged some changes that was in conflict with your changes, sorry about that.

To you plan to reopen another pull request?

I propose that you make two different pull requests:

  1. A refactor on the parent link between test methods, i.e. management of the map ampTestToParent.
  2. A refactor on the counter for Input Amplification and Assertions Amplifications

We can still discuss solutions.

Cheers, Benjamin.

@sbihel

This comment has been minimized.

Copy link
Contributor Author

@sbihel sbihel commented Mar 20, 2018

Hej @danglotb,

I was thinking on how to solve 2. and forgot to create the PR for 1.

So I thought we could have a counter of amplifications, and a list of exact changes made to the test method.

  1. To update the counter I would have liked to use something like Python's decorators for the apply method of Amplifier. It would have been great to easily get the size of the amplified methods list and add that to the counter. And if I implemented a classification for amplifiers (e.g. add, remove, or modify) we could also add that information to the counter.
  2. Instead of directly modify the AST with Spoon we could use an interface to log changes. This interface could have 3 functions: add node, modify node, and remove node.

Cheers, Simon.

@danglotb

This comment has been minimized.

Copy link
Member

@danglotb danglotb commented Mar 20, 2018

It would have been great to easily get the size of the amplified methods list and add that to the counter.

You can have this information by modifying Amplification object. You just need to break the stream, the map and you have it.

Instead of directly modify the AST with Spoon we could use an interface to log changes. This interface could have 3 functions: add node, modify node, and remove node.

I like the idea. How do you plan to support all the kind of insertion/deletion/modification?

What it come in my mind is a visitor pattern, but maybe you have something else.

I'm glad to hear that you are still on it!

Best,

-- Benjamin.

@sbihel

This comment has been minimized.

Copy link
Contributor Author

@sbihel sbihel commented Mar 21, 2018

You can have this information by modifying Amplification object. You just need to break the stream, the map and you have it.

Ah yes, thanks, I was trying to do something more complex than necessary 😅


What it come in my mind is a visitor pattern, but maybe you have something else.

There is a new change listener [1] in Spoon 🎉

[1] http://spoon.gforge.inria.fr/mvnsites/spoon-core/apidocs/spoon/experimental/modelobs/ActionBasedChangeListener.html


I'm glad to hear that you are still on it!

Sadly, I have come to the conclusion that my internship will not make progress unless I work on it...

💪

@danglotb

This comment has been minimized.

Copy link
Member

@danglotb danglotb commented Apr 10, 2018

Hi @sbihel

Any update?

@sbihel

This comment has been minimized.

Copy link
Contributor Author

@sbihel sbihel commented Apr 13, 2018

Not yet but I hope to show you something next week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.