Closed GoogleCodeExporter closed 9 years ago
Original comment by bestchai
on 18 Apr 2012 at 7:29
Original comment by jenny.abrahamson
on 18 Apr 2012 at 7:32
Addressed in revisions c10701e3f464 - dcad37bddb57, please review.
Original comment by jenny.abrahamson
on 25 Apr 2012 at 12:19
I left comments on revision c10701e3f464. I don't know how revision
dcad37bddb57 is relevant to this issue.
The end-to-end test you've added is a good start. But, the test relies on a
complex log, and does not do an exact check of the model (e.g., a bug could add
a trace to this model, and your test would not detect this). As well, it
depends on an external logfile, making it more difficult to debug in the future.
A good end-to-end test would be self contained and exact. A simple log, like
[(a,b), (a,a,b,b)] with just 2-3 events would suffice.
Also, please:
1. Do not mark an issue as Fixed until it is merged into default.
2. In the issue comments mention revisions by using "revision ID" syntax, so
that they are linked and easily accessible.
Original comment by bestchai
on 25 Apr 2012 at 12:53
I addressed your comments on revision c10701e3f464, but left in the dependence
on the external logfile. One idea to remove that dependence is to construct the
TraceParser in the test file from a local array of strings as in the
KTailInvariantMiner unit tests and add a new method in InvariMintMain that runs
InvariMint after accepting a ChainsTraceGraph. Is this alright, or would it
ruin the point of this being an "integration" test?
Original comment by jenny.abrahamson
on 27 Apr 2012 at 11:13
[deleted comment]
Issue 247 and Issue 248 document the external file dependency in InvariMint
tests and also propose a solution. Let's leave this task to those two issues.
Your changes improve the code, but you are still missing tests that would test
for exact models equivalence. Testing that a generated model accepts 1, 2, or 3
specific sequences is a good heuristic, but InvariMint may have subtle bugs
that may accept those strings as well as every other permutation of those
strings (e.g., intersection of NFby on their own gives you that).
I think you need a stronger set of tests that test the generated model for
equality against a model that is designed by hand and is known to be correct. A
few such simple tests (simple = just a few event types!) are enough, but
without them it is hard to be convinced that the algorithm is correct.
Original comment by bestchai
on 28 Apr 2012 at 12:27
Testing for equality with known models addressed in revision 5d76a1bc74a8
(specifically in InvariMint/src/tests/integration/EndToEndKTailsMainTest.java
and InvariMint/src/tests/integration/EndToEndMainTest.java)
Original comment by jenny.abrahamson
on 30 May 2012 at 7:14
Closing this while cleaning up open issues.
Original comment by jenny.abrahamson
on 5 Mar 2013 at 9:51
Original issue reported on code.google.com by
bestchai
on 24 Feb 2012 at 3:10