Code review comment for lp:~javier.collado/mago/suite_discovery

Revision history for this message
Eitan Isaacson (eeejay) wrote :

> Hello Eitan,
>
> Thanks for your comments and your feedback. I'm afraid that the code is not
> free from defects so I really appreciate your help.
>
> I've taken a look at that piece of code and the original version still looks
> fine to me. Let me explain it as clear as possible:
> - suite is a SuiteData object that implements the __eq__ method for
> comparisons
> - suite.name is the name of the test suite as written in the xml file
> - suite.filename is the name of the xml file that contains the suite
> description data
>
> My understanding was that in the main branch the suite filtering isn't
> performed by the suite name, but by the suite filename (with or without
> extension). Is that correct?

This has been correct. The main problem is that the "-i" option does not show suite files, but suite names. This defeats the purpose of the suites being discoverable, a user should be able to do this:

$ ubuntu-desktop-test -i
*suite/case list*
$ ubuntu-desktop-test -c <suite from list above>

I think both options should be available, by file or by suite. The suite filter is the most easy to use, but the file filter will allow things to not break when more than one suite have the same name.

> Please let me know if this is the way suite filtering is expected to work?

overriding __eq__ is fine, just see above.

> Anyway, I'll take a look at the safe_run_command output to make sure what
> happened to the logs.

That was my mistake, the logger is fine. The suite was simply not being run because I was supplying suite names, not files, see above.

« Back to merge proposal