Skip to content

Add codec unit tests - #13

Merged
sbuss merged 16 commits into
multiformats:masterfrom
fredthomsen:addCodecUnitTests
Jun 27, 2016
Merged

Add codec unit tests#13
sbuss merged 16 commits into
multiformats:masterfrom
fredthomsen:addCodecUnitTests

Conversation

@fredthomsen

Copy link
Copy Markdown
Contributor

Started breaking this out into another test file. Not done yet, but I just wanted to see if you were good with this.

@sbuss

sbuss commented May 17, 2016

Copy link
Copy Markdown
Collaborator

I just wanted to see if you were good with this.

Sounds good to me!

@fredthomsen

Copy link
Copy Markdown
Contributor Author

Alright I think this is ready now.

@sbuss

sbuss commented Jun 5, 2016

Copy link
Copy Markdown
Collaborator

@fredthomsen Sorry for the delay! I just started a new job so I haven't had much free time. I'm reviewing this now.

Comment thread tests/test_codec.py Outdated


@pytest.mark.parametrize("buf, expected", [
(b'047f0000011104d2047f0000010610e1', [b'\x04\x7f\x00\x00\x01', b'\x11\x04\xd2', b'\x04\x7f\x00\x00\x01', b'\x06\x10\xe1']), # "/ip4/127.0.0.1/udp/1234/ip4/127.0.0.1/tcp/4321"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: Please break this up onto multiple lines. I haven't yet set up a lint checker on travis, but I use flake8 locally to ensure a uniform style. If you could, please run make clean lint on your branch and correct any lint complaints. (the clean is because I slightly misconfigured flake8 and it'll try to lint pyc files if present)

@fredthomsen fredthomsen Jun 5, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I use the flake8 plugin in vim, but I tend to ignore line warnings... lol.

What is the best way to deal with a long string that exceeds 80 characters? I see a number of different methods suggested online and not sure that you have a preference.

@sbuss

sbuss commented Jun 5, 2016

Copy link
Copy Markdown
Collaborator

@fredthomsen Thanks very much for the tests! Sorry, again, for the delay reviewing them. The only problems I see are just lint/style issues.

@codecov-io

codecov-io commented Jun 22, 2016

Copy link
Copy Markdown

Current coverage is 94.94%

Merging #13 into master will increase coverage by 4.60%

@@             master        #13   diff @@
==========================================
  Files             5          5          
  Lines           352        356     +4   
  Methods           0          0          
  Messages          0          0          
  Branches         61         62     +1   
==========================================
+ Hits            318        338    +20   
+ Misses           21         10    -11   
+ Partials         13          8     -5   

Powered by Codecov. Last updated by 6973e9c...1da6127

@fredthomsen

fredthomsen commented Jun 22, 2016

Copy link
Copy Markdown
Contributor Author

@sbuss should multi_addr.util.int_to_hex return a binary string? What's the right approach here? I am having some issues.

EDIT: I think I resolved this.

@fredthomsen

Copy link
Copy Markdown
Contributor Author

Ok... really ready this time.

@sbuss

sbuss commented Jun 27, 2016

Copy link
Copy Markdown
Collaborator

This is excellent, @fredthomsen! Thanks for doing this!

@sbuss
sbuss merged commit 2a7a2e1 into multiformats:master Jun 27, 2016
@acul71 acul71 mentioned this pull request Jul 16, 2026
1 task
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