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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unleash the treefrog #11

Merged
merged 11 commits into from
Dec 5, 2018
Merged

Conversation

lqd
Copy link
Member

@lqd lqd commented Jul 13, 2018

It could be time, for the 馃惛 to leap from its tree and join us.
Worst case, that leap is optimal!

:)

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Code looks good to me! I took the liberty of pushing some additional cargo fmt + travis etc stuff into this PR, plus a rebase or two.

@frankmcsherry
Copy link
Contributor

馃憤

@frankmcsherry
Copy link
Contributor

Do you have merge rights, or should I click the big "merge pull request" button for you?

@nikomatsakis
Copy link
Contributor

I have merge rights; I was going to let travis finish.

@nikomatsakis
Copy link
Contributor

@frankmcsherry btw have you given any thought to how to do automated testing? I was contemplating trying to generate random data and some kind of "mega naive merge" algorithm and use that to create unit tests of the various join combinators.

@nikomatsakis
Copy link
Contributor

Possible mega naive join would be something like:

let mut join = source1.clone();
join.extend(source2.clone());
join.sort();
join.dedup();

@frankmcsherry
Copy link
Contributor

I have given it thought, and not come up with anything especially good. At least, over in more general "differential" space, I've not had great ideas about how to cover the sketchy cases with confidence. What I've done over there is try to do something like Graydon's philosophy that when you land commits you also put in tests that break before but not afterwards. These were more perf-oriented than correctness, though. =/

@nikomatsakis
Copy link
Contributor

OK. I figured I'd start off with a base of various "smoke tests" that use random data like I suggested, and then we'd add regression tests for specific bugs that are found (as you suggest).

@nikomatsakis nikomatsakis mentioned this pull request Dec 5, 2018
@frankmcsherry
Copy link
Contributor

That seems sane. One should be able to write fuzzing tests with random data, I'd bet, relating e.g. treefrog, naive datafrog, and brute-force, but I'm not sure about the confidence (e.g. would cover random data, but not random queries which are more strongly bound to the code itself).

@nikomatsakis
Copy link
Contributor

Indeed. I opened #15 for further discussion.

@nikomatsakis nikomatsakis merged commit 58a7cb3 into rust-lang:master Dec 5, 2018
@lqd lqd deleted the unleash-the-treefrog branch December 5, 2018 11:03
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.

None yet

3 participants