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

feat(mocks): change MockHttpBackend to define assertions on flush. #1206

Closed
wants to merge 2 commits into from

Conversation

vsavkin
Copy link
Contributor

@vsavkin vsavkin commented 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 #900, #1417

@vsavkin
Copy link
Contributor Author

vsavkin commented Jul 3, 2014

Some notes:

I've added flushGET, flushPOST, etc.

These new functions add a new expectation and then flush all the pending requests until they find the one matching the expectation.

    hb.when("GET", '/first').respond("OK");

    hb("GET", '/first', callback);
    hb("GET", '/second', callback);
    hb("GET", '/third', callback);

    hb.flushGET("GET", "/second").respond("OK"); // flushes "/first" and "/second"
    hb.flushGET("GET", "/third").respond("OK"); // flushes "/third"

I haven't deleted the expect function, so you can still write:

    hb.expect("GET", '/first').respond("OK");
    hb("GET", '/first', callback);
    hb.flush();

I can delete expect if you think it is not needed any more.

@vsavkin vsavkin added cla: yes and removed cla: no labels Jul 3, 2014
@jbdeboer
Copy link
Contributor

jbdeboer commented Jul 7, 2014

Awesome; could you add a second commit to the PR converting expects to flushGETs in the tests?

@mhevery mhevery added api: new and removed api: new labels Jul 21, 2014
@jbdeboer
Copy link
Contributor

This is a breaking change, so we need an upgrade plan.

Setting this to "reviewing" until then.

@vsavkin
Copy link
Contributor Author

vsavkin commented Aug 26, 2014

@jbdeboer This will cause issues only if there are tests that depend on the fact that the exception won't be thrown until flush. I can run the tests to see if it causes issues. If it does, we can easily change the tests to call flush.

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 force-pushed the 900_assertions_on_flush branch from f92ce6e to 5e25f1f Compare August 26, 2014 16:00
@jbdeboer
Copy link
Contributor

@vsavkin is reviewing this change and how it effects apps.

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

Successfully merging this pull request may close these issues.

Change assertion apis for http backend to define the assertion on flush
4 participants