Hi Saurabh,
On Wed, 2010-04-28 at 12:32 +0530, Saurabh Vyas wrote:
> Also I have added the test case, but I am not too sure that I have
> written the test case correctly, so please advice me
> if I need to write that in diff way (I have used test case similar to
> 'pkgsend add file' test case with imaginary file)
Did you actually run the tests? I applied your patch, compiled the
source and ran:
t...@igyo[287] env | grep saurabh
PATH=/export/home/timf/projects/ips/saurabh-test-pkg.hg/proto/root_i386/usr/bin:/usr/bin:/usr/sbin:/usr/X11/bin:/export/home/timf/bin:/export/home/timf/script:/opt/SunStudioExpress/bin/
PWD=/export/home/timf/projects/ips/saurabh-test-pkg.hg/src/tests
PYTHONPATH=/export/home/timf/projects/ips/saurabh-test-pkg.hg/proto/root_i386/usr/lib/python2.6/vendor-packages
SRC=/export/home/timf/projects/ips/saurabh-test-pkg.hg/proto/..
t...@igyo[288] pfexec ./run.py -o test_10_bundle_dir
# Accessibility not enabled, GUI tests disabled.
# logging to /tmp/tmpLMwnsJ.pkg-test.log
F
======================================================================
FAIL: cli.t_pkgsend.py TestPkgsendBasics.test_10_bundle_dir
----------------------------------------------------------------------
Traceback (most recent call last):
File "./pkg5unittest.py", line 328, in run
testMethod()
File "./api/../cli/t_pkgsend.py", line 524, in test_10_bundle_dir
rc, out3 = self.pkgsend(url, "generate %s" % imaginary_bundle)
File "./pkg5unittest.py", line 1441, in pkgsend
retcode, out, comment)
UnexpectedExitCodeException:
Invoked: cd /tmp/ips.test.10503;
/export/home/timf/projects/ips/saurabh-test-pkg.hg/proto/root_i386/usr/bin/pkgsend
-s http://localhost:12001 generate /tmp/ips.test.10503/imaginary_bundle
Expected exit status: 0. Got: 1. Output Follows:
,---------------------------------------------------------------------
| $ cd /tmp/ips.test.10503;
/export/home/timf/projects/ips/saurabh-test-pkg.hg/proto/root_i386/usr/bin/pkgsend
-s http://localhost:12001 generate /tmp/ips.test.10503/imaginary_bundle
| pkgsend: generate: '/tmp/ips.test.10503/imaginary_bundle' file not found
|
`---------------------------------------------------------------------
You need the call to self.pkgsend(..) on line 524 to include a parameter
"exit=1".
Having fixed that, the test still fails, this time with
Traceback (most recent call last):
File "./pkg5unittest.py", line 328, in run
testMethod()
File "./api/../cli/t_pkgsend.py", line 541, in test_10_bundle_dir
self.__validate_bundle_dir_package(pfmri, expected)
File "./api/../cli/t_pkgsend.py", line 89, in __validate_bundle_dir_package
self.assertEqual(len(exp_actions), len(actual))
AssertionError: 22 != 2
which looks quite suspicious to me, gen_actions(..) isn't doing what it
used to be with your change. I haven't dug into this any further - can
you take a look please?
cheers,
tim
> Thanks,
> ~Saurabh
>
> Brock Pytlik wrote:
> > On 04/23/10 02:22 AM, Saurabh Vyas wrote:
> >> Tim Foster wrote:
> >>> Hi Saurabh,
> >>>
> >>> On Thu, 2010-04-22 at 16:06 +0530, Saurabh Vyas wrote:
> >>>> Bug-id : pkgsend generate traceback when bundle doesn't exist
> >>>> webrev : http://cr.opensolaris.org/~saurabhv/fix-15646/
> >>>
> >>> Rather than just printing an error message inside gen_actions() and
> >>> continuing, you should find a way to have trans_generate() detect the
> >>> error and return a non-zero exit code back to main_func()
> >>>
> >>> I'd suggest raising an exception from gen_actions for this case,
> >>> handling it in trans_generate(), printing the exception message, then
> >>> returning a non-zero exit code.
> >> Thanks for the inputs Tim.
> >>
> >> So now I am raising pkg.actions.ActionDataError from gen_actions()
> >> and handling this in trans_generate() (with non zero exit)
> >> Is pkg.actions.ActionDataError right error to use or should another
> >> error type needs to be defined ?
> >>
> >> Just for ref : http://cr.opensolaris.org/~saurabhv/fix-15646-rev-1/
> >>
> >
> > I think it's probably worth using the error function on line 473
> > instead of just printing the error, see line 445 for an example. It'd
> > be nice if you could attach an example of what the output looks like,
> > though depending on how you write your test, that could be sufficient.
> >
> >> Question : Can't we directly handle this error in gen_actions()
> >> itself? (as we know that we cannot recover from this error & should
> >> quit with zero zero code).
> >>
> > We try to keep the user interaction bits of code all confined to one
> > layer. For example, suppose we decided to create a GUI version of
> > pkgsend. We'd want it to use gen_actions so that we didn't duplicate
> > code. However, having gen_actions print something to a terminal (which
> > might not exist) then call sys.exit(1) wouldn't allow a particularly
> > useful interface to the user.
> >
> > Brock
> >
> >
> >>> You also need to include a test case for this bug
> >>> in ./src/tests/cli/t_pkgsend.py
> >> I will add the test case and send the final webrev soon.
> >>
> >> Thanks,
> >> ~Saurabh
> >>> cheers,
> >>> tim
> >>>
> >>>
> >>
> >> _______________________________________________
> >> pkg-discuss mailing list
> >> [email protected]
> >> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
> >
> > _______________________________________________
> > pkg-discuss mailing list
> > [email protected]
> > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>
> _______________________________________________
> pkg-discuss mailing list
> [email protected]
> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
