Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Change assertion apis for http backend to define the assertion on flush #900

Closed
IgorMinar opened this issue Apr 14, 2014 · 5 comments
Closed
Milestone

Comments

@IgorMinar
Copy link
Contributor

instead of

backend.expectGET('/foo').respond('some response');
// do stuff
backend.flush()

do

// do stuff
backend.flushGET('/foo', 'some response')

Benefits:

  • makes tests much easier to follow
  • reduces the amount of code developer needs to write
  • gives control over the order in which requests should be flushed
@mhevery
Copy link
Contributor

mhevery commented Apr 16, 2014

👍

@mhevery mhevery added this to the Mocks milestone Apr 16, 2014
@vsavkin
Copy link
Contributor

vsavkin commented Jun 16, 2014

I'd like to work on it

@vsavkin
Copy link
Contributor

vsavkin commented Jun 20, 2014

I looked a bit into it. To implement this kind of API, MockHttpBackend will have to change in a significant way.

Right now MockHttpBackend throws when you call it with an unexpected request.

backend("GET", /some"); //throws here if `/some` has not been defined

To make the API change possible MockHttpBackend will have to run this check when you call flush.

backend("GET", /some"); //no problem, just records the request
backend.flush(); //throws here

This is a breaking change.

If the breaking change is not an option, there is another way to make tests more readable, by introducing some sort of autoFlush option.

backend.autoFlush = true; //it's done once somewhere in a beforeEach

backend.expect('GET', '/some').respond(200, 'success');
backend("GET", /some"); //calls the response function right away. So no need to call `flush`.

@IgorMinar
Copy link
Contributor Author

We've been looking into this on the JS side as well and concluded that the
breaking change is too big for v1.x.

I however urge you to make this change before cutting v1.0. The benefits of
this change on usability and readability of the test code are huge.

On Fri, Jun 20, 2014 at 9:50 AM, Victor Savkin [email protected]
wrote:

I looked a bit into it. To implement this kind of API, MockHttpBackend
will have to change in a significant way.

Right now MockHttpBackend throws when you call it with an unexpected
request.

backend("GET", /some"); //throws here if /some has not been defined

To make the API change possible MockHttpBackend will have to run this
check when you call flush.

backend("GET", /some"); //no problem, just records the request
backend.flush(); //throws here

This is a breaking change.

If the breaking change is not an option, there is another way to make
tests more readable, by introducing some sort of autoFlush option.

backend.autoFlush = true; //it's done once somewhere in a beforeEach

backend.expect('GET', '/some').respond(200, 'success');
backend("GET", /some"); //calls the response function right away. So no need to call flush.


Reply to this email directly or view it on GitHub
#900 (comment)
.

@vsavkin
Copy link
Contributor

vsavkin commented Jun 25, 2014

Yup, it does make tests easier to follow.

I can submit a PR with the change if it is OK with everyone.

vsavkin added a commit to vsavkin/angular.dart that referenced this issue Jul 3, 2014
Add new methods (`flushGET`, `flushPOST`, etc) to the `MockHttpBackend` class to allow defining assertions on flush. These methods add a new expectation and then flush all the pending requests until they find the one matching the expectation.

BREAKING CHANGE:

Unexpected requests are detected only when `flush` is called.

Before:

		backend("GET", /some"); //throws here if `/some` has not been defined

After:

		backend("GET", /some"); //no problem, just records the request
		backend.flush(); //throws here

The signature of `flush` has changed.

Before:

		`flush([int count])`

After:

		`flush({int count, Function condition})`

Closes dart-archive#900
vsavkin added a commit to vsavkin/angular.dart that referenced this issue Jul 3, 2014
Add new methods (`flushGET`, `flushPOST`, etc) to the `MockHttpBackend` class to allow defining assertions on flush. These methods add a new expectation and then flush all the pending requests until they find the one matching the expectation.

BREAKING CHANGE:

Unexpected requests are detected only when `flush` is called.

Before:

		backend("GET", /some"); //throws here if `/some` has not been defined

After:

		backend("GET", /some"); //no problem, just records the request
		backend.flush(); //throws here

Closes dart-archive#900
vsavkin added a commit to vsavkin/angular.dart that referenced this issue Jul 7, 2014
Add new methods (`flushGET`, `flushPOST`, etc) to the `MockHttpBackend` class to allow defining assertions on flush. These methods add a new expectation and then flush all the pending requests until they find the one matching the expectation.

BREAKING CHANGE:

Unexpected requests are detected only when `flush` is called.

Before:

		backend("GET", /some"); //throws here if `/some` has not been defined

After:

		backend("GET", /some"); //no problem, just records the request
		backend.flush(); //throws here

Closes dart-archive#900
vsavkin added a commit to vsavkin/angular.dart that referenced this issue Aug 26, 2014
Add new methods (`flushGET`, `flushPOST`, etc) to the `MockHttpBackend` class to allow defining assertions on flush. These methods add a new expectation and then flush all the pending requests until they find the one matching the expectation.

BREAKING CHANGE:

Unexpected requests are detected only when `flush` is called.

Before:

		backend("GET", /some"); //throws here if `/some` has not been defined

After:

		backend("GET", /some"); //no problem, just records the request
		backend.flush(); //throws here

Closes dart-archive#900
@vsavkin vsavkin closed this as completed in 0a8ce12 Sep 5, 2014
vsavkin added a commit to vsavkin/angular.dart that referenced this issue Sep 7, 2014
Add new methods (`flushGET`, `flushPOST`, etc) to the `MockHttpBackend` class to allow defining assertions on flush. These methods add a new expectation and then flush all the pending requests until they find the one matching the expectation.

BREAKING CHANGE:

Unexpected requests are detected only when `flush` is called.

Before:

		backend("GET", /some"); //throws here if `/some` has not been defined

After:

		backend("GET", /some"); //no problem, just records the request
		backend.flush(); //throws here

Closes dart-archive#900
vsavkin added a commit to vsavkin/angular.dart that referenced this issue Sep 7, 2014
Add new methods (`flushGET`, `flushPOST`, etc) to the `MockHttpBackend` class to allow defining assertions on flush. These methods add a new expectation and then flush all the pending requests until they find the one matching the expectation.

BREAKING CHANGE:

Unexpected requests are detected only when `flush` is called.

Before:

		backend("GET", /some"); //throws here if `/some` has not been defined

After:

		backend("GET", /some"); //no problem, just records the request
		backend.flush(); //throws here

Closes dart-archive#900
vsavkin added a commit to vsavkin/angular.dart that referenced this issue Sep 8, 2014
Add new methods (`flushGET`, `flushPOST`, etc) to the `MockHttpBackend` class to allow defining assertions on flush. These methods add a new expectation and then flush all the pending requests until they find the one matching the expectation.

BREAKING CHANGE:

Unexpected requests are detected only when `flush` is called.

Before:

		backend("GET", /some"); //throws here if `/some` has not been defined

After:

		backend("GET", /some"); //no problem, just records the request
		backend.flush(); //throws here

Closes dart-archive#900
vsavkin added a commit that referenced this issue Sep 11, 2014
Add new methods (`flushGET`, `flushPOST`, etc) to the `MockHttpBackend` class to allow defining assertions on flush. These methods add a new expectation and then flush all the pending requests until they find the one matching the expectation.

BREAKING CHANGE:

Unexpected requests are detected only when `flush` is called.

Before:

		backend("GET", /some"); //throws here if `/some` has not been defined

After:

		backend("GET", /some"); //no problem, just records the request
		backend.flush(); //throws here

Closes #900
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants