[BEAM-6904] Add tests for structuralValue implementation in coders #8208
Conversation
|
@kennknowles Could you please review this changes? |
kennknowles
left a comment
kennknowles
left a comment
There was a problem hiding this comment.
Thanks! This is very good with one change suggested.
sdks/java/core/src/test/java/org/apache/beam/sdk/coders/BigIntegerCoderTest.java
Show resolved
Hide resolved
|
@kennknowles I added checks for non-equal members |
|
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions. |
|
@kennknowles Whats the status on this? |
|
Ah, my mistake. I forgot to actually "request" my own review so it was not showing in my review queue. |
|
These tests are definitely useful. I want to merge it. I will take a minute and see if I can resolve the conflicts. Since it is caused by my delay, please allow me and I will push back to this branch if I have perms. |
|
Ah, and the conflicts are caused by a prior commit being reverted because of issues with Dataflow pipeline update. |
|
Indeed, we need to roll forward #8258 for this one. |
|
I don't want to waste you time, @AlexKbit. It seems that a lot of these tests are for methods that were rolled back. So if you want to submit just the tests that apply to existing implementations, that's cool. But maybe you would prefer to wait. The issue with update incompatibility in Dataflow needs to get resolved anyhow. |
|
Ok I think I can wait or add patch to ticket in Jira. |
|
It is OK with me you can leave this open or close it. Might as well keep the useful code waiting until the right time. |
|
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
3 participants
Hi @kennknowles !
This change contains tests on structuralValue implementation for base coders (with deprecate consistentWithEquals)
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.