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

[FLINK-2479] Refactor runtime.operators.* tests #1160

Closed
wants to merge 4 commits into from

Conversation

zentol
Copy link
Contributor

@zentol zentol commented Sep 21, 2015

This PR refactors most runtime.operators tests to use the TupleGenerator introduced in FLINK-2105.

The changes made can generally be summed up as replacing usages of Records with Tuple2<Integer, String> and adjusting serializers/comparators accordingly. I've added a few static helper methods to the TestData class to simplify comparator/serializer(factory) creation.

One oddball change is the introduction of a separate IntPairComparator whose hash() implementation uses the underlying IntComparators hash() method, to be in line with the TupleComparators implementation.

@StephanEwen
Copy link
Contributor

That is very nice, thanks for the initiative, @zentol

Hope to be able to have a look over it, soon. Looks in principle correct, but cannot hurt to have another pair of eyes check for subtle mistakes...

@zentol
Copy link
Contributor Author

zentol commented Sep 24, 2015

I will shortly add another commit that removes the remaining Record portions in the TestData class and refactors its usages. Originally i planned a separate PR for those, but it took a lot less time than expected.

@@ -725,7 +718,7 @@ public void join(IntPair rec1, Record rec2, Collector<Record> out) throws Except
}
}

static final class IntPairRecordPairComparator extends TypePairComparator<IntPair, Record>
static final class IntPairRecordPairComparator extends TypePairComparator<IntPair, Tuple2<Integer, String>>
Copy link
Contributor

Choose a reason for hiding this comment

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

should be IntPairTuplePairComparator

@fhueske
Copy link
Contributor

fhueske commented Oct 6, 2015

Hi @zentol, thanks for this effort!
I found a few classes that could be renamed as well. Otherwise, the PR looks good.
Thanks

@zentol
Copy link
Contributor Author

zentol commented Oct 13, 2015

I've renamed the classes and rebased the PR.

@fhueske
Copy link
Contributor

fhueske commented Oct 15, 2015

Thanks for the update @zentol!
I will merge this PR.

@asfgit asfgit closed this in fbc18b9 Oct 15, 2015
cfmcgrady pushed a commit to cfmcgrady/flink that referenced this pull request Oct 23, 2015
lofifnc pushed a commit to lofifnc/flink that referenced this pull request Oct 23, 2015
@zentol zentol deleted the 2479_testdata_pr branch November 4, 2015 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants