Skip to content

(tests) - act should be able to track and flush the full queue and all subsequent queues#1520

Merged
marvinhagemeister merged 8 commits into
masterfrom
testing/effects
Apr 12, 2019
Merged

(tests) - act should be able to track and flush the full queue and all subsequent queues#1520
marvinhagemeister merged 8 commits into
masterfrom
testing/effects

Conversation

@JoviDeCroock

@JoviDeCroock JoviDeCroock commented Apr 7, 2019

Copy link
Copy Markdown
Member

While reasoning about our current testing approach I found a critical flaw. What happens when for example you type in an input -> this triggers a validation -> this sets state. A series of hooks get executed and well our current approach in act has no way of following these up.

This is why I introduced a global array in options, named effects. Every queued effect will be pushed and will be removed when we're invoking/cleaning up. This makes for a consistent testing method.

@JoviDeCroock JoviDeCroock marked this pull request as ready for review April 7, 2019 13:12
@coveralls

coveralls commented Apr 7, 2019

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling cb90afc on testing/effects into ef01a85 on master.

@JoviDeCroock JoviDeCroock changed the title (tests) - act should be able to track and flush the full queue (tests) - act should be able to track and flush the full queue and all subsequent queues Apr 7, 2019
@robertknight

Copy link
Copy Markdown
Member

Great. I ran into this recently and had to implement a utility as a workaround. I'll try to test this out locally and see if it resolves the issue in my case.

@JoviDeCroock

Copy link
Copy Markdown
Member Author

@robertknight that would be awesome, please do! It works for the case I ran into, if you find another one just hit me up and we can add it in tests.

@marvinhagemeister marvinhagemeister left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, it's getting better and better 👍 🎉

@marvinhagemeister marvinhagemeister merged commit 5966511 into master Apr 12, 2019
@marvinhagemeister marvinhagemeister deleted the testing/effects branch April 12, 2019 12:19
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.

4 participants