edemo / PDEngine

voting engine using ADA authentication and condorcet method
Other
2 stars 12 forks source link

Feature/cast vote records the vote cast 60 #264

Closed valentinbujdoso closed 5 years ago

valentinbujdoso commented 5 years ago

I did something very bad, or somebody setted more strict the sonar?! :O

magwas commented 5 years ago

Sonar settings did not change. I take a deeper look.

magwas commented 5 years ago

PIT says there is no coverage for isUpdateVotesCast and most of addCastVote I don't see a call for isUpdateVotesCast anywhere, and you might ony call addCastVote when Vote.votesCast is empty. Do you TDD? https://adadocs.demokracia.rulez.org/PDEngine/edemo/480/pit-reports/201903161744/org.rulez.demokracia.pdengine/Vote.java.html

valentinbujdoso commented 5 years ago

Yes I did TDD, I just forget to delete isUpdateVotesCast function. However if I run in my local the coverage looks good.

magwas commented 5 years ago

using "coverage as" in eclipse, or running the full CI in testenv and looking at the pit report?

valentinbujdoso commented 5 years ago

I meant the eclipse coverage run. However I think I find out. The problem is this: On Vote.java:107

    for (int i = 0; i < votesCast.size() && !isModified; i++) {

The sonar say it is partially covered by test (3/4).

Cases: votesCast empty votesCast not empty isModified false isModified true

However I have tests all of the above mentioned cases.

magwas commented 5 years ago

I have told Sonar again not to whine about entity fields, but you have two zombies to kill.

valentinbujdoso commented 5 years ago
Screenshot 2019-03-17 at 22 17 54

It is very strange, because in my local with eclipse coverage as... get the 4/4 branches result. You can see the attached printscreen.

valentinbujdoso commented 5 years ago

Finally, I did it. :)

valentinbujdoso commented 5 years ago

@magwas I think I did everything what you asked. :)

valentinbujdoso commented 5 years ago

@magwas Could you please review this? There is some issue which is blocked by this issue.

magwas commented 5 years ago

there are conflicts to be resolved here.

valentinbujdoso commented 5 years ago

@magwas I think I fixed everything what you asked.

magwas commented 5 years ago

testcaseCount=12