Ticket #401 (closed cage: fixed)

Opened 5 years ago

Last modified 4 years ago

pbc tools have inadequate test coverage

Reported by: cotto Owned by:
Priority: normal Milestone:
Component: none Version:
Severity: medium Keywords: testing
Cc: Language:
Patch status: Platform:

Description

pbc_dump and pbc_disassemble were broken for the time between r37063 and r37113. They're working (afaict), but it disturbs me that breaking these tools didn't cause any failures in make test (or make fulltest).

If these tools are worth maintaining (and I suspect so), they need to get some increased test coverage. I was able to segfault both tools by running them on pbc_to_exe.pbc in Parrot's root dir, though they both worked fine against a the pbc generated from a pir hello world. It does appear that both tools have some basic coverage, but we could really do better, especially given all the pbc that's generated as part of Parrot's build process.

Unfortunately I'm not sure how to make this into a closeable ticket. A good start would be that make test should fail if any of the following lines are changed to something goofy:

  • the GETATTR_Key_next_key(inteprp, key, key) lines in src/packdump.c
  • the GETATTR_Key_next_key(interp, k, k); in src/debug.c

Anything too far beyond that will require someone with more knowledge of these tools than me.

Attachments

trac101-t-tools.patch Download (11.1 KB) - added by rurban 5 years ago.
r34575-tt101-add-parrot_utils-tests.diff Download (11.4 KB) - added by rurban 5 years ago.
r34567-tt97-test-installables.diff Download (3.9 KB) - added by rurban 5 years ago.

Change History

Changed 5 years ago by rurban

Changed 5 years ago by rurban

Changed 5 years ago by rurban

  Changed 5 years ago by allison

Reini,

Like I said before, what you need to do to get these patches applied is expand them into full test files. A "proof-of-concept" file with one test in it won't be applied. (Attaching the same unmodified patches to a different ticket isn't going to improve their chances of being applied. The general policy is to link to the other ticket like:)

See TT #101 and TT #97 for an earlier start on solving this problem.

Allison

in reply to: ↑ description   Changed 5 years ago by jkeenan

Replying to cotto:

> 
> If these tools are worth maintaining (and I suspect so), 
> they need to get some increased test coverage.  

I suspect that there are other people on list in addition to me who have little familiarity with these tools.  So let me ask some basic questions:

* What is their purpose?  Who would be likely to use them, and how?

* Where are they documented?

* To the best of your knowledge, what tests do we have so far?

* How would we measure the extent to which the tests cover either the code itself or these tools' specifications?

Thank you very much.[[BR]]
kid51

  Changed 4 years ago by tcurtis

  • keywords testing added

  Changed 4 years ago by cotto

  • status changed from new to closed
  • resolution set to fixed

We have some tests for pbc_dump and pbc_disassemble thanks to dukeleto++. Since it's now straightforward to add additional tests for specific bugs in these tools (and since the basic cases are covered), I'm closing this ticket.

Note: See TracTickets for help on using tickets.