You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/04/12 16:33:40 UTC
[GitHub] [beam] KevinGG opened a new pull request, #17351: [BEAM-14288] Fixed flaky test
KevinGG opened a new pull request, #17351:
URL: https://github.com/apache/beam/pull/17351
Use assertIn instead of assertEqual when asserting formatted dict
because the order of accessing entries in a dict is non-deterministic.
This includes repr(x: dict).
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #17351: [BEAM-14288] Fixed flaky test
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on PR #17351:
URL: https://github.com/apache/beam/pull/17351#issuecomment-1096987992
> because the order of accessing entries in a dict is non-deterministic.
Note that Python dictionaries are [ordered since Python 3.6](https://stackoverflow.com/questions/39980323/are-dictionaries-ordered-in-python-3-6) - I don't think this is the root cause.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] commented on pull request #17351: [BEAM-14288] Fixed flaky test
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #17351:
URL: https://github.com/apache/beam/pull/17351#issuecomment-1096974802
# [Codecov](https://codecov.io/gh/apache/beam/pull/17351?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17351](https://codecov.io/gh/apache/beam/pull/17351?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b9cc0b5) into [master](https://codecov.io/gh/apache/beam/commit/2e0f79ef2b924da3306d0e55dffd4728809b9f3d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2e0f79e) will **decrease** coverage by `0.00%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #17351 +/- ##
==========================================
- Coverage 73.94% 73.93% -0.01%
==========================================
Files 684 684
Lines 89519 89519
==========================================
- Hits 66193 66190 -3
- Misses 22166 22169 +3
Partials 1160 1160
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `83.63% <ø> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/17351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...python/apache\_beam/runners/worker/worker\_status.py](https://codecov.io/gh/apache/beam/pull/17351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvd29ya2VyX3N0YXR1cy5weQ==) | `78.26% <0.00%> (-1.45%)` | :arrow_down: |
| [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/17351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `92.25% <0.00%> (-0.81%)` | :arrow_down: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/17351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.06% <0.00%> (+0.15%)` | :arrow_up: |
| [sdks/python/apache\_beam/transforms/combiners.py](https://codecov.io/gh/apache/beam/pull/17351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb21iaW5lcnMucHk=) | `93.42% <0.00%> (+0.38%)` | :arrow_up: |
| [sdks/python/apache\_beam/internal/metrics/metric.py](https://codecov.io/gh/apache/beam/pull/17351/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvbWV0cmljcy9tZXRyaWMucHk=) | `93.00% <0.00%> (+1.00%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17351?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/17351?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [2e0f79e...b9cc0b5](https://codecov.io/gh/apache/beam/pull/17351?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] KevinGG commented on pull request #17351: [BEAM-14288] Fixed flaky test
Posted by GitBox <gi...@apache.org>.
KevinGG commented on PR #17351:
URL: https://github.com/apache/beam/pull/17351#issuecomment-1097102158
> > because the order of accessing entries in a dict is non-deterministic.
>
> Note that Python dictionaries are [ordered since Python 3.6](https://stackoverflow.com/questions/39980323/are-dictionaries-ordered-in-python-3-6) - I don't think this is the root cause.
Yes, you are right. I just checked some CPython source [code](https://github.com/python/cpython/blob/474fdbe9e4a2ff90ef39e8748da644c86a200981/Lib/typing.py#L269)
So the deduplication logic when building a Union type accesses parameters through a set. And since set is not ordered, this is where the flakiness come. I'll update the comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on a diff in pull request #17351: [BEAM-14288] Fixed flaky test
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on code in PR #17351:
URL: https://github.com/apache/beam/pull/17351#discussion_r848899324
##########
sdks/python/apache_beam/runners/interactive/sql/utils_test.py:
##########
@@ -76,17 +76,23 @@ def test_replace_single_pcoll_token(self):
replaced_sql, 'SELECT * FROM PCOLLECTION WHERE a=1 AND b=2')
def test_pformat_namedtuple(self):
- self.assertEqual(
- 'ANamedTuple(a: int, b: str)', pformat_namedtuple(ANamedTuple))
+ expected = pformat_namedtuple(ANamedTuple)
+ self.assertIn('ANamedTuple(', expected)
+ self.assertIn('a: int', expected)
+ self.assertIn('b: str', expected)
- @unittest.skip("BEAM-14288")
def test_pformat_namedtuple_with_unnamed_fields(self):
- self.assertEqual(
- f'OptionalUnionType(unnamed: {repr(Optional[Union[int, str]])})',
- pformat_namedtuple(OptionalUnionType))
+ expected = pformat_namedtuple(OptionalUnionType)
+ self.assertIn('OptionalUnionType(unnamed: ', expected)
+ self.assertIn('Union', expected)
+ self.assertIn('int', expected)
+ self.assertIn('str', expected)
+ self.assertIn('NoneType', expected)
Review Comment:
Using assertIn seems a little brittle (e.g. `Union[int,my_custom_string_type,float64,NoneType]` would pass as well), but I'm probably being nitpicky here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] KevinGG commented on pull request #17351: [BEAM-14288] Fixed flaky test
Posted by GitBox <gi...@apache.org>.
KevinGG commented on PR #17351:
URL: https://github.com/apache/beam/pull/17351#issuecomment-1096948098
R: @TheNeuralBit
PTAL, thx!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] robertwb merged pull request #17351: [BEAM-14288] Fixed flaky test
Posted by GitBox <gi...@apache.org>.
robertwb merged PR #17351:
URL: https://github.com/apache/beam/pull/17351
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] KevinGG commented on a diff in pull request #17351: [BEAM-14288] Fixed flaky test
Posted by GitBox <gi...@apache.org>.
KevinGG commented on code in PR #17351:
URL: https://github.com/apache/beam/pull/17351#discussion_r848918025
##########
sdks/python/apache_beam/runners/interactive/sql/utils_test.py:
##########
@@ -76,17 +76,23 @@ def test_replace_single_pcoll_token(self):
replaced_sql, 'SELECT * FROM PCOLLECTION WHERE a=1 AND b=2')
def test_pformat_namedtuple(self):
- self.assertEqual(
- 'ANamedTuple(a: int, b: str)', pformat_namedtuple(ANamedTuple))
+ expected = pformat_namedtuple(ANamedTuple)
+ self.assertIn('ANamedTuple(', expected)
+ self.assertIn('a: int', expected)
+ self.assertIn('b: str', expected)
- @unittest.skip("BEAM-14288")
def test_pformat_namedtuple_with_unnamed_fields(self):
- self.assertEqual(
- f'OptionalUnionType(unnamed: {repr(Optional[Union[int, str]])})',
- pformat_namedtuple(OptionalUnionType))
+ expected = pformat_namedtuple(OptionalUnionType)
+ self.assertIn('OptionalUnionType(unnamed: ', expected)
+ self.assertIn('Union', expected)
+ self.assertIn('int', expected)
+ self.assertIn('str', expected)
+ self.assertIn('NoneType', expected)
Review Comment:
I think it's fine for these functions since they just pretty print stuff. I could also enumerate all possible combinations of the parameters and then assertIn those.
Sending a commit to reverse test changes for dict and use enumerated values for asserting non-deterministic formatting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] KevinGG commented on pull request #17351: [BEAM-14288] Fixed flaky test
Posted by GitBox <gi...@apache.org>.
KevinGG commented on PR #17351:
URL: https://github.com/apache/beam/pull/17351#issuecomment-1097164381
Run Python PreCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org