Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test todo title getting undefined after edit #46

Closed
wants to merge 2 commits into from

Conversation

hgwood
Copy link

@hgwood hgwood commented Aug 7, 2017

Closes #39 Adds a test or the scenario described in issue #39.

I took the opportunity to add a minimal test harness based on tape.

@staltz
Copy link
Member

staltz commented Aug 7, 2017

Hi @hgwood :)
Thanks for submitting this PR, we appreciate any kind of desire to help. Before merging this PR could you notice that the latest version of this TodoMVC is under the branch onionify. We should merge it to master soon, when onionify v4 is launched. Could you make this PR against that branch? I'm not even sure if the bug would still occur in the branch.

@hgwood
Copy link
Author

hgwood commented Aug 7, 2017

Hello @staltz (should have said that first, sorry). Thank you for your kind reply and pointers. After a quick look at the code, it seems the bug is indeed fixed in the onionify branch. I will confirm by rebasing and running the test later. Would you still be interested in merging the test into onionify?

@hgwood hgwood force-pushed the fix/done-edit-updates-title branch from 05126e6 to 3b25e8b Compare August 8, 2017 19:34
@hgwood hgwood changed the base branch from master to onionify August 8, 2017 19:34
@hgwood hgwood changed the title Fix todo title getting undefined after edit Test todo title getting undefined after edit Aug 8, 2017
@hgwood hgwood changed the title Test todo title getting undefined after edit [WIP] Test todo title getting undefined after edit Aug 8, 2017
@hgwood hgwood force-pushed the fix/done-edit-updates-title branch from 3b25e8b to cad2a85 Compare August 8, 2017 20:05
@hgwood hgwood changed the title [WIP] Test todo title getting undefined after edit Test todo title getting undefined after edit Aug 8, 2017
@hgwood
Copy link
Author

hgwood commented Aug 8, 2017

I have changed the test to match the new app. It does show the bug is fixed.

@hgwood hgwood force-pushed the fix/done-edit-updates-title branch from 575c396 to 280f116 Compare August 8, 2017 20:11
@staltz
Copy link
Member

staltz commented Oct 31, 2017

Hi @hgwood, thanks for the interesting test to keep this bug from returning. (I'm going through dozens of issues and PRs)

I would merge this, but as a single isolated test it may feel odd to someone browsing the source code when learning this. So I'll leave this PR open, you can choose whether to close it or add more comprehensive tests so that the whole codebase is tested.

@hgwood
Copy link
Author

hgwood commented Nov 8, 2017

I understand. I will close it in favor of other priorities. Thank you for your consideration.

@hgwood hgwood closed this Nov 8, 2017
@hgwood hgwood deleted the fix/done-edit-updates-title branch November 8, 2017 12:35
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.

2 participants