> On March 20, 2015, 5:59 p.m., Zameer Manji wrote:
> > src/test/python/apache/aurora/client/cli/test_supdate.py, line 101
> > <https://reviews.apache.org/r/32313/diff/1/?file=901501#file901501line101>
> >
> >     Using a raw mock here is a little bit dangerous if the shape of the raw 
> > config changes underneath the test. Would you mind setting the spec 
> > argument of the Mock class here? 
> > http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock
> >     
> >     I was thinking of something like Mock(spec=['has_cron_schedule']). 
> > Alternatively you could set the spec to be the Job schema object. I think 
> > it could be of the form of Mock(spec=aurora.config.schema.base.Job) or 
> > Mock(spec=aurora.config.schema.base.Job()). This might be impossible 
> > because Job() is a pystachio object but I strongly suggest investigating it.
> >     
> >     This way if we try to access other attributes the test will fail.

I ventured down this path, but it appears that pystachio is too dynamic for 
this to be possible [1].  I'll do the spec_set routine, but i fear this will be 
untenable in other scenarios.

[1] 
https://github.com/wickman/pystachio/blob/master/pystachio/composite.py#L205-L211


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32313/#review77248
-----------------------------------------------------------


On March 20, 2015, 5:26 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32313/
> -----------------------------------------------------------
> 
> (Updated March 20, 2015, 5:26 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1206
>     https://issues.apache.org/jira/browse/AURORA-1206
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> One change i snuck in here is in `cli/__init__.py`.  This makes the 
> subcommand help include the the description:
> 
> ```
> $ aurora update info -h
> usage: aurora update info [-h] [--write-json] [--verbose]
>                           [--skip-hooks hook,hook,...]
>                           CLUSTER/ROLE/ENV/NAME [ID]
> 
> Display detailed status information about a scheduler-driven in-progress
> update. If no update ID is provided, information will be displayed about the
> active update for the job.
> 
> positional arguments:
>   CLUSTER/ROLE/ENV/NAME
>                         Fully specified job key, in CLUSTER/ROLE/ENV/NAME
>                         format
>   ID                    Update identifier provided by the scheduler when an
>                         update was started.
> 
> optional arguments:
>   -h, --help            show this help message and exit
>   --write-json          Generate command output in JSON format
>   --verbose, -v         Show verbose output
>   --skip-hooks hook,hook,...
>                         A comma-separated list of command hook names that
>                         should be skipped. If the hooks cannot be skipped,
>                         then the command will be aborted
> ```
> 
> Prior to this change, the description was only displayed in the parent 
> command's help text.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/__init__.py 
> 6a0c129bc5d5dac8d8d393705a69586c9918983d 
>   src/main/python/apache/aurora/client/cli/update.py 
> 830ef4424fe46bc8c14456492f29dea681cf5200 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> f9acbdfd65adb252f3059717a6bc1a1f4ba39c44 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> 320c1fbeee0161528745edd38360cd1fd5d53104 
> 
> Diff: https://reviews.apache.org/r/32313/diff/
> 
> 
> Testing
> -------
> 
> I have converted all test cases in `test_supdate.py` to use the 'new style' 
> non-integration testing, which removed a ton of boilerplate.
> 
> I also corrected some holes in the end-to-end tests, wherein `test` and 
> conditions could silently fail.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to