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

Add iter, keys, and values with Java tests #2

Merged
merged 3 commits into from
Jan 21, 2020
Merged

Conversation

thallada
Copy link
Contributor

I was following part of you stream earlier today and decided to try my hand at porting some of the other Java tests from MapCheck.java.

The contains_key tests were pretty straightforward since you already implemented contains_key. Those are t4 and t7.

During the stream, you mentioned that NodeIter is a private Iterator which could be the basis for public iterators on keys, values, and entries. I've done that in a new file src/iter/iter.rs with three new structs Iter, Keys, and Values (similar to std::collections::HashMap). I added some of my own tests for the new iter(), keys(), and values() methods and ported ittest1, ittest2, and ittest3 from MapCheck.java.

To get the values out of the NodeIter, I had to add two more unsafe blocks.

I'm still a noob at concurrency and Rust so let me know if there's anything to improve here. I'm using this as a learning experience. Thanks for streaming!

src/iter/iter.rs Outdated Show resolved Hide resolved
@jonhoo
Copy link
Owner

jonhoo commented Jan 19, 2020

I love this change, thank you! Could you rebase onto master so that you get the CI testing changes I just pushed?

@thallada thallada force-pushed the master branch 2 times, most recently from ec33e28 to 47944c1 Compare January 21, 2020 04:23
@codecov
Copy link

codecov bot commented Jan 21, 2020

Codecov Report

Merging #2 into master will increase coverage by 2.26%.
The diff coverage is 95.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #2      +/-   ##
==========================================
+ Coverage    83.1%   85.37%   +2.26%     
==========================================
  Files           5        6       +1     
  Lines         586      670      +84     
==========================================
+ Hits          487      572      +85     
+ Misses         99       98       -1
Impacted Files Coverage Δ
src/lib.rs 80.33% <100%> (+1.35%) ⬆️
src/iter/iter.rs 91.89% <91.89%> (ø)
tests/jdk/map_check.rs 98.43% <97.36%> (-1.57%) ⬇️
src/iter/traverser.rs 95.96% <0%> (+1.61%) ⬆️

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 437663f...dcb9c44. Read the comment docs.

@thallada
Copy link
Contributor Author

Rebased and added docs. Let me know if this needs anything else!

@jonhoo jonhoo merged commit 1f168f4 into jonhoo:master Jan 21, 2020
@jonhoo
Copy link
Owner

jonhoo commented Jan 21, 2020

Excellent, thank you!

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