Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement assertions for network primitives #133

Merged
merged 7 commits into from
Aug 28, 2020

Conversation

christian-bromann
Copy link
Member

@christian-bromann christian-bromann commented Jul 28, 2020

After we landed network primitives in WebdriverIO our next step is to add proper expect matchers making the assertion easy. This patch adds:

const mock = browser.mock('https://todo-backend-express-knex.herokuapp.com/')
expect(mock).toBeRequested() // similar to expect(mock).toBeCalledTimes(1)
expect(mock).toBeRequestedTimes(12)
expect(mock).toBeRequestedWithResponse({ foo: 'bar' })

@codecov
Copy link

codecov bot commented Jul 28, 2020

Codecov Report

Merging #133 into master will increase coverage by 0.67%.
The diff coverage is 67.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
+ Coverage   64.37%   65.05%   +0.67%     
==========================================
  Files          35       39       +4     
  Lines         553      724     +171     
  Branches      104      155      +51     
==========================================
+ Hits          356      471     +115     
- Misses        187      242      +55     
- Partials       10       11       +1     
Impacted Files Coverage Δ
src/matchers.ts 100.00% <ø> (ø)
src/util/expectAdapter.ts 44.44% <ø> (ø)
src/jasmineUtils.ts 53.27% <53.27%> (ø)
src/matchers/mock/toBeRequested.ts 100.00% <100.00%> (ø)
src/matchers/mock/toBeRequestedTimes.ts 100.00% <100.00%> (ø)
src/matchers/mock/toBeRequestedWithResponse.ts 100.00% <100.00%> (ø)
src/utils.ts 87.69% <100.00%> (+1.53%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53bfbba...d85f963. Read the comment docs.

@mgrybyk
Copy link
Member

mgrybyk commented Jul 29, 2020

toBeCalled matcher overwrites jests builtin one.

@christian-bromann
Copy link
Member Author

toBeCalled matcher overwrites jests builtin one.

Would that be a problem, given that users aren't likely to use stubbing of functions in e2e test context? We could also name it toBeRequested, toBeRequestedTimes and toBeRequestedWithResponse - what do you think?

@christian-bromann
Copy link
Member Author

what do you think?

Actually I like these names much more .. I will update it.

@mgrybyk
Copy link
Member

mgrybyk commented Jul 29, 2020

Matcher names looks good, I'll proceed reviewing later today

@mgrybyk
Copy link
Member

mgrybyk commented Jul 29, 2020

Will it be possible to verify not just body but headers, url as well?

@christian-bromann
Copy link
Member Author

@mgrybyk added feature that allows toBeRequestedTimes to take number options:

expect(mock).toBeRequestedTimes(1)
// is the same as
expect(mock).toBeRequestedTimes({ eq: 1 })
// also possible
expect(mock).toBeRequestedTimes({ lte: 2 })
expect(mock).toBeRequestedTimes({ gte: 2 })

Error message is:

Expect mock to be called times
Expected: ">= 3"
Received: 0

What do you think?

@mgrybyk
Copy link
Member

mgrybyk commented Aug 26, 2020

Looks good, let's fix the failing tests and merge

@christian-bromann
Copy link
Member Author

The tests are passing, not sure why the push check fails. I will merge and check if master passes.

@christian-bromann christian-bromann merged commit fdec3f0 into master Aug 28, 2020
@christian-bromann christian-bromann deleted the cb-mock-matchers branch August 28, 2020 08:32
This was referenced Aug 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants