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

Tests for HandledNode #546

Merged
merged 51 commits into from
Nov 2, 2018
Merged

Conversation

dvdplm
Copy link
Contributor

@dvdplm dvdplm commented Oct 9, 2018

This is ready for a review. I have left a ton of debugging println! in the code as I expect this to need some work before it's ready to merge: I think a few of these tests are very brittle so if someone have ideas on how to do better, please speak up. :)

Let me know if you notice any important code paths that are not tested, it's easy to miss some.

dvdplm added 16 commits October 3, 2018 11:16
…e-listeners

* upstream/master:
  Implement GET_PROVIDERS/ADD_PROVIDER Kademlia messages (libp2p#530)
  Make secio almost compile for asmjs/wasm (libp2p#519)
  Make it possible to accept/deny nodes in CollectionsStream (libp2p#512)
  Fix polling in handled_node.rs (libp2p#531)
  Remove notifying tasks (libp2p#528)
  Fix handler not properly shut down, and write test for this (libp2p#514)
  Use unsigned-varints, add BLAKE2 support in multihash (libp2p#525)
  Upgrade smallvec version to fix license issue (libp2p#526)
…e-listeners

* upstream/master:
  Add unit tests for core::nodes::NodeStream (libp2p#535)
…e-listeners

* upstream/master:
  Fix secio compied with --no-default-features (libp2p#545)
  multiaddr: explain the use of `&str` for `Unix`. (libp2p#543)
  Model HandshakeContext with explicit state transitions. (libp2p#532)
  multiaddr: add support for onion protocol. (libp2p#542)
.gitignore Outdated
@@ -1,2 +1,4 @@
target
Cargo.lock
.idea/**
CMakeLists.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

@dvdplm dvdplm self-assigned this Oct 10, 2018
…andled_node

* dp/chore/test-core-listeners:
  Prefer freestanding function
  Address grumbles
  Remove non-project specific stuff
  Address grumbles
Add HandlerState and use it in TestBuilder
Shorter test names
Track events as they reach the Handler
Describe complex test logic
…e-handled_node

* upstream/master:
  Fix the polling process in handled node (libp2p#582)
  Fix panicking when Kad responder is destroyed (libp2p#575)
  Remove other unused files (libp2p#570)
  Fix panic in raw swarm (libp2p#571)
  Remove two unused files (libp2p#567)
…e-handled_node

* upstream/master:
  Remove even more unused files (libp2p#581)
…e-handled_node

* upstream/master:
  Add protocol to report external address view. (libp2p#566)
  Add a TransportExt trait (libp2p#533)
  libp2p#399 remove tokio_current_thread tests (libp2p#577)
Copy link
Member

@tomaka tomaka left a comment

Choose a reason for hiding this comment

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

Please also remove the printlns now.

}
fn inject_event(&mut self, inevent: Self::InEvent) {
self.events.push(inevent)
}
Copy link
Member

Choose a reason for hiding this comment

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

Indentation.

Also this repo uses spaces and rustfmt.
You can use rustfmt but please only commit this file, otherwise you'll conflict with the entire world.

// – …and causes the Handler to yield Async::Ready(None)
// – which in turn makes the HandledNode to yield Async::Ready(None) as well
assert_matches!(handled.poll(), Ok(Async::Ready(None)));
assert_eq!(handled.handler.events, vec![
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should access private fields. The tests should only use the public API.
However I think it would be fine to add handler(&self) and handler_mut(&mut self) methods to HandledNode.

@tomaka
Copy link
Member

tomaka commented Nov 1, 2018

CI reports a legit parsing error.

@tomaka tomaka merged commit 437a8c0 into libp2p:master Nov 2, 2018
dvdplm added a commit to dvdplm/rust-libp2p that referenced this pull request Nov 2, 2018
…_swarm

* upstream/master:
  Add a IdentifyTransport (libp2p#569)
  Tests for HandledNode (libp2p#546)
  Some minor fixes reported by clippy (libp2p#600)
dvdplm added a commit to dvdplm/rust-libp2p that referenced this pull request Nov 2, 2018
…e-handled_node_tasks

* upstream/master:
  Add a IdentifyTransport (libp2p#569)
  Tests for HandledNode (libp2p#546)
  Some minor fixes reported by clippy (libp2p#600)
  Add a PeriodicIdentification protocol handler (libp2p#579)
  Add ProtocolsHandler trait (libp2p#573)
  Use paritytech/rust-secp256k1 (libp2p#598)
  Use websocket 0.21.0 (libp2p#597)
  Reexport multihash from the facade (libp2p#587)
  Add substrate to the list of projects using libp2p (libp2p#595)
  Remove spaces before semicolons (libp2p#591)
dvdplm added a commit to dvdplm/rust-libp2p that referenced this pull request Nov 9, 2018
…lection_stream

* upstream/master:
  Use vecdeque for fair polling (libp2p#610)
  Add back the Swarm (libp2p#613)
  Remove dependency on tokio_current_thread (libp2p#606)
  Use chashmap from crates (libp2p#615)
  Fix grammar in core/nodes etc. (libp2p#608)
  Inject event by value in ProtocolsHandler (libp2p#605)
  Add a PeriodicPingHandler and a PingListenHandler (libp2p#574)
  Fix stack overflow when printing a SubstreamRef (libp2p#599)
  Add a peer id generator (libp2p#583)
  eg. -> e.g.; ie. -> i.e. via repren (libp2p#592)
  Add a IdentifyTransport (libp2p#569)
  Tests for HandledNode (libp2p#546)
  Some minor fixes reported by clippy (libp2p#600)
  Add a PeriodicIdentification protocol handler (libp2p#579)
  Add ProtocolsHandler trait (libp2p#573)
  Use paritytech/rust-secp256k1 (libp2p#598)
  Use websocket 0.21.0 (libp2p#597)
  Reexport multihash from the facade (libp2p#587)
  Add substrate to the list of projects using libp2p (libp2p#595)
  Remove spaces before semicolons (libp2p#591)
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