Skip to content

Preserve source map key order for torrent files#6

Merged
dampcake merged 2 commits intodampcake:masterfrom
TheLQ:master
Jul 15, 2019
Merged

Preserve source map key order for torrent files#6
dampcake merged 2 commits intodampcake:masterfrom
TheLQ:master

Conversation

@TheLQ
Copy link
Contributor

@TheLQ TheLQ commented Jul 7, 2019

Simple change to keep the source order of keys in maps. Useful for

@coveralls
Copy link

coveralls commented Jul 7, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 8a587de on TheLQ:master into 52abad9 on dampcake:master.

@dampcake dampcake self-requested a review July 8, 2019 15:23
@dampcake dampcake self-assigned this Jul 8, 2019
@dampcake dampcake added this to the 1.2.2 milestone Jul 8, 2019
Copy link
Owner

@dampcake dampcake left a comment

Choose a reason for hiding this comment

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

I think adding some tests around the ordering expectations makes sense too.

@SuppressWarnings("unchecked")
public void testWriteDictionary() throws Exception {
instance.writeDictionary(new HashMap<Object, Object>() {{
instance.writeDictionary(new TreeMap<Object, Object>() {{
Copy link
Owner

Choose a reason for hiding this comment

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

I think it makes more sense to use a LinkedHashMap and modify the expectation.
Will probably make the tests a little easier to read too!

@codecov-io
Copy link

codecov-io commented Jul 15, 2019

Codecov Report

Merging #6 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #6      +/-   ##
============================================
- Coverage     99.47%   99.46%   -0.01%     
+ Complexity       86       85       -1     
============================================
  Files             7        7              
  Lines           191      188       -3     
  Branches         28       27       -1     
============================================
- Hits            190      187       -3     
  Partials          1        1
Impacted Files Coverage Δ Complexity Δ
.../java/com/dampcake/bencode/BencodeInputStream.java 100% <100%> (ø) 26 <0> (ø) ⬇️
...java/com/dampcake/bencode/BencodeOutputStream.java 100% <100%> (ø) 23 <3> (-1) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74a55e7...02a992f. Read the comment docs.

@TheLQ
Copy link
Contributor Author

TheLQ commented Jul 15, 2019

Added those tests. Unless you think otherwise, the TreeMap ordering test was just removed instead of copied to a new test.

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