/juju

api/base/testing: simplify API call checkers #7222

Merged
merged 1 commit into from Apr 19, 2017

Conversation

Reviewers

@mjs mjs

@mhilton mhilton

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@rogpeppe @jujubot @mjs @mhilton
@rogpeppe
Owner

rogpeppe commented Apr 10, 2017

With the upcoming changes to command mocking, we're going to be
using API call checkers quite a bit more, so simplify the API
and make it a little more general. Specifically, we

  • integrate CheckingAPICallerMultiArgs and CheckingAPICaller into one
  • make NotifyingCheckingAPICaller just do notification
    on an APICaller implementation without mixing that with the checking
    logic
  • make the error return a part of an individual call rather
    than separate.
  • include the call count in the returned APICaller implementation
    rather than a separate argument.

QA No regressions.
@mhilton

mhilton approved these changes Apr 10, 2017
View changes

LGTM

@rogpeppe
api/base/testing: simplify API call checkers 
With the upcoming changes to command mocking, we're going to be
using API call checkers quite a bit more, so simplify the API
and make it a little more general. Specifically, we

- integrate CheckingAPICallerMultiArgs and CheckingAPICaller into one
- make NotifyingCheckingAPICaller just do notification
on an APICaller implementation without mixing that with the checking
logic
- make the error return a part of an individual call rather
than separate.
- include the call count in the returned APICaller implementation
rather than a separate argument.
2d5b67c
@mjs

mjs approved these changes Apr 19, 2017
View changes

Nice improvements.

@rogpeppe
Owner

rogpeppe commented Apr 19, 2017

$$merge$$
@jujubot
Contributor

jujubot commented Apr 19, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 8fdc25e into juju:develop Apr 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment