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

Refactor parent map #374

Merged
merged 7 commits into from Mar 26, 2018

Conversation

@sbihel
Copy link
Contributor

sbihel commented Mar 20, 2018

Address #364

sbihel and others added 5 commits Mar 19, 2018
Doc
@danglotb

This comment has been minimized.

Copy link
Member

danglotb commented Mar 26, 2018

Hi @sbihel

Thank you again for the PR. It seems that we still have the same issues.

After investigation, the problem comes from the fact that we modify the CtMethod used as a key.

We clone the given CtMethod to modify it. Since we put the key just after clone, and before modification, the HashMap computes the hashcode of the unmodified clone and so when we want to get using the modified clone, no hashcode match, and we obtain a NPE.

A simple solution is to use an IdentityHashMap rather than a HashMap.

I just opened a PR on your branch, see sbihel#1

I am gonna review the rest of your PR after the merging.

@danglotb danglotb self-requested a review Mar 26, 2018
@danglotb danglotb self-assigned this Mar 26, 2018
Use identity map
@sbihel

This comment has been minimized.

Copy link
Contributor Author

sbihel commented Mar 26, 2018

Aaah I see. Silly me

Thanks a lot!

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 26, 2018

Pull Request Test Coverage Report for Build 961

  • 47 of 51 (92.16%) changed or added relevant lines in 12 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 84.849%

Changes Missing Coverage Covered Lines Changed/Added Lines %
dspot/src/main/java/fr/inria/diversify/dspot/selector/JacocoCoverageSelector.java 10 11 90.91%
dspot/src/main/java/fr/inria/diversify/dspot/selector/PitMutantScoreSelector.java 1 2 50.0%
dspot/src/main/java/fr/inria/diversify/utils/AmplificationHelper.java 17 19 89.47%
Totals Coverage Status
Change from base Build 948: -0.05%
Covered Lines: 3713
Relevant Lines: 4376

💛 - Coveralls
@@ -290,7 +319,8 @@ public static String getClassPath(DSpotCompiler compiler, InputConfiguration con
final Long average = average(valuesToMethod.keySet());
while (reducedTests.size() < MAX_NUMBER_OF_TESTS) {
final Long furthest = furthest(valuesToMethod.keySet(), average);
reducedTests.add(valuesToMethod.get(furthest).get(0));
CtMethod<?> discardedTest = valuesToMethod.get(furthest).get(0);
reducedTests.add(discardedTest);

This comment has been minimized.

Copy link
@danglotb

danglotb Mar 26, 2018

Member

The name is wrongly chosen. In the opposite that happen: we keep the "furthest" method. That is why I removed your call to remove() on the map of parents. I think we can inline the variable here.

This comment has been minimized.

Copy link
@sbihel

sbihel Mar 26, 2018

Author Contributor

I have to be more focused. Sorry

This comment has been minimized.

Copy link
@danglotb

danglotb Mar 26, 2018

Member

No you good, That's the whole point of the review process :)

@danglotb danglotb merged commit fc0eb26 into STAMP-project:master Mar 26, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.05%) to 84.849%
Details
@sbihel sbihel deleted the sbihel:clean_amplifier2 branch Mar 26, 2018
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.