[GitHub] [beam] zhoufek commented on a change in pull request #15603: [BEAM-9487] Various Trigger.may_lose_data fixes

[GitBox <gi...@apache.org>]

zhoufek commented on a change in pull request #15603:
URL: https://github.com/apache/beam/pull/15603#discussion_r723714559



##########
File path: sdks/python/apache_beam/transforms/ptransform_test.py
##########
@@ -505,12 +504,10 @@ def test_group_by_key_allow_unsafe_triggers(self):
           | beam.Create([(1, 1), (1, 2), (1, 3), (1, 4)])
           | WindowInto(
               window.GlobalWindows(),
-              trigger=trigger.AfterCount(5),
+              trigger=trigger.AfterCount(4),
               accumulation_mode=trigger.AccumulationMode.ACCUMULATING)
           | beam.GroupByKey())
-      # We need five, but it only has four - Displays how this option is
-      # dangerous.
-      assert_that(pcoll, is_empty())
+      assert_that(pcoll, equal_to([(1, [1, 2, 3, 4])]))

Review comment:
       Yes, but that doesn't currently happen. If I set it to five, I get:
   
   ```
   apache_beam.testing.util.BeamAssertException: Failed assert: [(1, [1, 2, 3, 4])] == [], missing elements [(1, [1, 2, 3, 4])] [while running 'assert_that/Match']
   ```
   
   I'm guessing there's  a bug  with the Direct Runner.




-- 
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