On 02/10/10 03:41 PM, Danek Duvall wrote:
Shawn Walker wrote:
http://cr.opensolaris.org/~swalker/pkg-9123/
...
file.py:
- line 190: What's an example of a mode that's an integer, but os.chmod()
will raise an EnvironmentError on?
You mean OSError? But yeah, it can't happen like I thought.
- line 205: why call validate() here, since validate() only looks at
mode, which isn't involved in this operation?
I was anticipating the addition of timestamp validation. I originally
didn't have this call here, and was torn over whether it was better to
validate into places where invalid attribute values could cause an
issue. But you're right that this feels premature, so I'll just remove
it for now (plus line 226).
imageplan.py:
- line 1103: shouldn't we do the same here as we're now doing for
pkg.actions.ActionErrors?
Are you saying this should be an "except BaseException, e:" and then
ensure we re-raise the original?
t_action.py:
- Probably should add a test explicitly for a non-octal mode.
As in "-rwxr-xr-x" or the like? Do we want to allow for and convert
that syntax?
Seems like there's a good deal of copied code between t_pkg_info.py,
t_pkg_install.py, and t_pkg_api_install.py. Can that be cleaned up?
At the moment, I can't think of a good place for it to live. It's
specific to this particular testing scenario, so it didn't seem right to
put it into pkg5unittest.py.
The only thing that comes to mind at the moment is adding a new test
suite that tests the bad package scenario for all of these operations
instead of doing it in each operation's test suite. I'm open to
suggestions...
Thanks for the review!
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
