On Tue 23 Jun 2009 at 05:15PM, Michal Pryc wrote:
> Hey,
> I have prepared yet another webrev for the automated tests for IPS GUI.
>
> Those two webrevs can be applied separately and are for one purpose, to
> make automated tests available for the IPS GUI.
>
My high level comment is that this set of wads needs more work.
I could be confused, but at a high level:
- ldtp.sh has no copyright or cddl.
- Don't write scripts in bash in our gate. It's POSIX sh, ksh,
or python.
- Don't we have code in setup.py to download stuff? Why
can't we use that?
- Would be it be better to just get JDS guys to add ldtp?
- To follow the convention I established, LDTP should
go into either src/extern or src/gui/extern.
- Why are you doing the build in a shell script? Isn't
that why we have make? How will you ensure we don't
keep remaking this?
DTrace scripts are lacking copyrights and licenses. A note that
something is "based on" something else is not sufficient.
run_dtrace.sh:
- similar complaints as above.
- RESULT_DIR should be setable from the environment. Directories
are directories, not folders
*.py: I'm pretty unhappy that we built a very nice test infrastructure
which you are not using. pkg5testcase should be able to supply a lot
of what you need or could be subclasses or extended to do what you
want. It avoids things like hardcoding localhost:10000 everywhere,
provides logging, and would make all of your tests discoverable by
'run.py'.
- __author__ should be eliminated (and it isn't even spelled
right in test_ipsgui_start.py)
- Test cases need copyright and license.
- Occasionally there is code commented out
(test_ipsgui_remove.py:42)
- wait(10), wait(5), wait(15) in test cases seems awful. Please
DO NOT commit this sin.
Thanks,
-dp
--
Daniel Price, Solaris Kernel Engineering http://blogs.sun.com/dp
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
