Add tests for refx REQUEST_POST_UPDATE effects network requests.

[BE-Webdesign]

These tests handle the network effects portion of unit testing our refx middleware. This makes use of stubs for testing the history API, and explicitly modifying the window.location object to be writable. For the network effects, these effects are broken out into seperate functions which use partial application to isolate the networkCallback. The postRequest function becomes a wrapper for wp.api.models.Post(). So the API remains consistent. Rather than doing a sinon mock, the wp.api.models.Post interface is manually mocked for easy future changing. As we change the actual implementation of the network requests these tests will fail, which is a good thing. If we use straight mocks we might get false positives instead.

Testing Instructions
Verify Trashing and Saving still works properly. Verify that tests pass.

[nylen]

You should be able to do // eslint-disable-line name-of-the-failing-rule instead.

[BE-Webdesign]

In addition to that, I am not sure whether this is a good approach for mocking the wp.api stuff. Any improved ideas would be very welcome!

[BE-Webdesign]

Think I might close this out. It kind of just makes our actual code more complicated than it should be.

[aduth]

I think we'd want some form of testing here, but I agree network mocking is a bit of a pain. It would be nicer if we could chain effects by dispatching or returning a generic type: 'REQUEST' action. That way we only need to test that the effects here only dispatch an action with expected properties, isolating the request logic to its own separate effect. This is, however, at odds with how we currently use wp.api, since it wouldn't be very easy to pass as action properties the chain we'd otherwise use to build a request with wp.api.

[gziolo]

Thanks for working on those tests 👍

Another approach to consider would be to stub method calls for whatever wp.api.models.Post constructor creates. Unfortunately I'm still learning the codebase, so can't help more in this case. The proposed unit tests require lots of boilerplate code and are a bit difficult to write. It would be great to find an easier way, as it seems we could reuse such pattern in other places.

[gziolo]

This tests depends on chai and sinon. We would have to rewrite them to work with Jest :)

@BE-Webdesign do you plan to take another look at this one?

[BE-Webdesign]

BE-Webdesign do you plan to take another look at this one?

Yup, eventually I will. I am waiting on the decision of the testing framework before getting back into the tests.

[gziolo]

Cool, thanks for your update. We should know more by the end of tomorrow. I'm betting on Jest after the upcoming relicensing :)

[BE-Webdesign]

@gziolo

Review would be great, this just tests the REQUEST_POST_UPDATE effect. Will make seperate PRs for the other pieces.

The tests verify that the proper actions are dispatched under the correct conditions.

[youknowriad]

Hey, @BE-Webdesign Good work here. But in general I try to compare the gain we'd have in having tests compared to the burden of maintaining them when adding tests. (I know some people like 100% coverage no matter the complexity of the tests, but I disagree with this approach in general).

So I'm wondering here, are these tests useful enough to deserve maintaining these complex tests (hard to follow, mocking api, promises...)? I'm leaning towards avoiding these but happy to be proven wrong.

[karmatosed]

As discussed in this week's meeting due to changes in way API requests are called.

[aduth]

Slack discussion: https://wordpress.slack.com/archives/C02QB2JS7/p1520432967000006

[aduth]

A few things:

I agree with @youknowriad's concerns at #966 (comment) , and really it speaks to a bigger issue with how we're doing side effects, as they're quite complex and overly convoluted. I think at the very least we should be targeting less the action type which is the source of the effect, and rather the individual behaviors themselves. This may just be extracting functions which are called by the effect. See #5417 as an example.

With some of the generalized patterns for network behaviors like explored in #5219, I hope we'll need less of these side effects in general.

Finally, with changes in #5253, the mocking here would not need to be adjusted significantly.