Skip to content

Conversation

nhunzaker
Copy link
Contributor

Reset and patch return an action. This PR sets those actions up so that they are rejected whenever deserializing their payloads fail.

Deserialize still raises an exception.

I've also added a new test helper for quickly testing repo state in a safe way.

toHaveStatus (action, status) {
if (action instanceof Action === false) {
throw new TypeError("toHaveStatus expects an Action. Received " +
(action ? "a " + action.constructor.name : action) + ".")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to return null if no action on the second ternary return value? Might be easier to read by explicitly having null there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to account for other values, like undefined, a string, or a number, or something else. This at least calls toString on it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah okay 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah.. hmm. I see your point though. It'll be confusing to read:

expect("foobar").toHaveStatus("closed")
toHaveStatus expects an Action. Received "foobar".

Instead of:

"toHaveStatus expects an Action. Received a String."

What's better in your opinion?

@mackermedia
Copy link
Contributor

👍

@nhunzaker nhunzaker force-pushed the deserialize-and-test-helpers branch from 630ae36 to 291dc8d Compare February 16, 2017 15:20
@nhunzaker nhunzaker merged commit 291dc8d into master Feb 16, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 291dc8d on deserialize-and-test-helpers into 5209f63 on master.

@nhunzaker nhunzaker deleted the deserialize-and-test-helpers branch February 27, 2017 14:04
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.

3 participants