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.

Sign up for GitHub

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

Jump to bottom

Testify refactor #56

Merged
merged 4 commits into from Apr 2, 2020
Merged

Testify refactor #56

merged 4 commits into from Apr 2, 2020

Conversation

hannahhoward
Copy link
Collaborator

Goals

Graphsync is big and has lots of tests. Those tests are not very easy to reach particularly because they involve a lot of tests on channels. This PR introduces Testify and a few utility functions to make the tests vastly more readable and clearer in intent.

Implementation

  • Introduce testify, replace standard if -> t.Fatal pattern -- means test that correct conditions are met, rather than that the opposite (incorrect conditions trigger an error) -- makes for more clear, readable code
  • Clean up error messages
  • Refactor a few tests to switch to simpler table test form, BDD
  • Introduce channel assertion utilities. Often we are testing for the fact that a value comes in on a channel before context cancellation, or one channel produces a value before others. We often need to read the value in the process. This produces a confusing pattern of select statements throughout the tests. While the channel assertions have to use some reflect fanciness internally to work for generic parameters (but with some type checking), it makes for vastly more readable assertions in the tests, making the intent clearer and the test way more concise

@hannahhoward
refactor(tests): introduce testify
1e6b08d 
Introduce testify, cleanup lots of tests, add channel assertions to simplify channel tests, make
more readable
@hannahhoward
refactor(tests): clean up error messages
9cf95fe
@hannahhoward
fix(deps): remove bitswap
c0e0f6a 
remove accidentally added bitswap
@hannahhoward
refactor(tests): put expected in correct position
db1e49e 
Make sure for require.equal that the expected value is the first value
@hannahhoward hannahhoward merged commit b3cc648 into master Apr 2, 2020
@aschmahmann aschmahmann mentioned this pull request Sep 22, 2020
Closed
72 tasks
@mvdan mvdan deleted the refactor/testify branch December 15, 2021 14:15
marten-seemann pushed a commit that referenced this pull request Mar 2, 2023
@hannahhoward
Minor fixes for retrieval on data transfer (#56)
34b009b 
* fix(types): cbor-gen for channel ID

* fix(impl): better voucher decoder

use both type registries to decode vouchers in channels

* refactor(channels): use go-statemachine GetSync

rather than clunky noopSynchronize event, use built in GetSync method in go-statemachine

* feat(channels): add channel id to channel state

* fix(impl): fix integration test

Not sure how this was managing to work given how it's written!

* fix(deps): update go-statemachine

* fix(impl): fix test stability

was failng in situations with where request completed, due to timing issues.

* fix(deps): update go-statemachine

* fix(impl): revalidation sends complete responses

make sure the revalidation sends a different message for a response that is finalizing versus
regular

* fix(fsm): allow progress at start

* fix(deps): update graphsync

* feat(transport): cleanup channels after complete

do not cleanup channels until transfer is definitely complete

* fix(deps): update go-graphsync to master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@hannahhoward