You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "lostluck (via GitHub)" <gi...@apache.org> on 2024/01/22 22:05:44 UTC

[PR] [#29917][prism] Initial TestStream support [beam]

lostluck opened a new pull request, #30072:
URL: https://github.com/apache/beam/pull/30072

   Adds initial TestStream support for Prism for #29917.
   
   * Handles watermark advancement.
   * Multiple Outputs (though, this is untested until covered by Python, as Go doesn't permit tag specification, however, the same lookup mechanism remains in the single output case)
   * Handles arbitrary element bytes.
   * Handles being consumed as a side input.
   
   A bit of light experimentation on handling design puts most test stream related code into a single file, and uses an interface to delegate the execution of events to the different events themeselves.
   
   The only easily testable logic outside of a pipeline execution context would be popping events from a queue. But as the alternative to correct popping is a pipeline freeze and crash (pending elements remaining, but nothing to trigger their processing), these are omitted.
   
   The only included logic that isn't currently covered is Processing Time handling, though the core is present in this PR. TestStreams with ProcessingTime events will be hard failed at job submission time for now.  This is no great loss as Prism doesn't meaningfully handle ProcessingTime events at all, and that will require it's own coordination to insert their executions in the queue.
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
    - [ ] 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://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-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)
   ------------------------------------------------------------------------------------------------
   [![Build python source distribution and wheels](https://github.com/apache/beam/workflows/Build%20python%20source%20distribution%20and%20wheels/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
   [![Python tests](https://github.com/apache/beam/workflows/Python%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Java tests](https://github.com/apache/beam/workflows/Java%20Tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
   [![Go tests](https://github.com/apache/beam/workflows/Go%20tests/badge.svg?branch=master&event=schedule)](https://github.com/apache/beam/actions?query=workflow%3A%22Go+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 or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
   


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1949226664

   Thank you kindly!


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1971988535

   Found them. 
   
   `--- FAIL: TestTestStreamInt16Sequence (10.52s)`
   
   Basically, the Flink runner does nonsense with TestStream whenever there's a LengthPrefix, leading to an *extra* length prefix being added to custom coders, strings, byte slices etc.
   
   We've never successfully found where/why this gets added.
   
   But ultimately it means the data is being corrupted, since the extra length prefixing occurs flink side, but the *coder* received back by the SDK doesn't include it, causing decode failures.
   
   The problem in this case is that I didn't filter out *all* the new tests I added too. I'll have a PR to fix shortly.


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1904914400

   ## [Codecov](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: `47 lines` in your changes are missing coverage. Please review.
   > Comparison is base [(`87145a8`)](https://app.codecov.io/gh/apache/beam/commit/87145a8c2c16bbaa1c19a84e1c0ac36bddd49b1b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 38.40% compared to head [(`d55d970`)](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 38.46%.
   > Report is 28 commits behind head on master.
   
   | [Files](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...m/runners/prism/internal/jobservices/management.go](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL3ByaXNtL2ludGVybmFsL2pvYnNlcnZpY2VzL21hbmFnZW1lbnQuZ28=) | 6.66% | [13 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...am/runners/prism/internal/engine/elementmanager.go](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL3ByaXNtL2ludGVybmFsL2VuZ2luZS9lbGVtZW50bWFuYWdlci5nbw==) | 50.00% | [12 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...g/beam/runners/prism/internal/engine/teststream.go](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL3ByaXNtL2ludGVybmFsL2VuZ2luZS90ZXN0c3RyZWFtLmdv) | 87.61% | [12 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [sdks/go/pkg/beam/runners/prism/internal/execute.go](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2Rrcy9nby9wa2cvYmVhbS9ydW5uZXJzL3ByaXNtL2ludGVybmFsL2V4ZWN1dGUuZ28=) | 70.83% | [6 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #30072      +/-   ##
   ==========================================
   + Coverage   38.40%   38.46%   +0.06%     
   ==========================================
     Files         697      698       +1     
     Lines      102261   102417     +156     
   ==========================================
   + Hits        39274    39399     +125     
   - Misses      61360    61389      +29     
   - Partials     1627     1629       +2     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/beam/pull/30072/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [go](https://app.codecov.io/gh/apache/beam/pull/30072/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `54.28% <72.35%> (+0.10%)` | :arrow_up: |
   
   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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/beam/pull/30072?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck merged PR #30072:
URL: https://github.com/apache/beam/pull/30072


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1905096040

   Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "damondouglas (via GitHub)" <gi...@apache.org>.
damondouglas commented on code in PR #30072:
URL: https://github.com/apache/beam/pull/30072#discussion_r1492794851


##########
sdks/go/pkg/beam/runners/prism/internal/engine/elementmanager.go:
##########
@@ -320,21 +331,27 @@ func (em *ElementManager) Bundles(ctx context.Context, nextBundID func() string)
 				}
 			}
 			if len(em.inprogressBundles) == 0 && len(em.watermarkRefreshes) == 0 {
-				v := em.livePending.Load()
-				slog.Debug("Bundles: nothing in progress and no refreshes", slog.Int64("pendingElementCount", v))
-				if v > 0 {
-					var stageState []string
-					ids := maps.Keys(em.stages)
-					sort.Strings(ids)
-					for _, id := range ids {
-						ss := em.stages[id]
-						inW := ss.InputWatermark()
-						outW := ss.OutputWatermark()
-						upPCol, upW := ss.UpstreamWatermark()
-						upS := em.pcolParents[upPCol]
-						stageState = append(stageState, fmt.Sprintln(id, "watermark in", inW, "out", outW, "upstream", upW, "from", upS, "pending", ss.pending, "byKey", ss.pendingByKeys, "inprogressKeys", ss.inprogressKeys, "byBundle", ss.inprogressKeysByBundle, "holds", ss.watermarkHoldHeap, "holdCounts", ss.watermarkHoldsCounts))
+				nextEvent := em.testStreamHandler.NextEvent()
+				if nextEvent == nil {
+					v := em.livePending.Load()
+					slog.Debug("Bundles: nothing in progress and no refreshes", slog.Int64("pendingElementCount", v))
+					if v > 0 {
+						var stageState []string
+						ids := maps.Keys(em.stages)
+						sort.Strings(ids)
+						for _, id := range ids {
+							ss := em.stages[id]
+							inW := ss.InputWatermark()
+							outW := ss.OutputWatermark()
+							upPCol, upW := ss.UpstreamWatermark()
+							upS := em.pcolParents[upPCol]
+							stageState = append(stageState, fmt.Sprintln(id, "watermark in", inW, "out", outW, "upstream", upW, "from", upS, "pending", ss.pending, "byKey", ss.pendingByKeys, "inprogressKeys", ss.inprogressKeys, "byBundle", ss.inprogressKeysByBundle, "holds", ss.watermarkHoldHeap, "holdCounts", ss.watermarkHoldsCounts))
+						}
+						panic(fmt.Sprintf("nothing in progress and no refreshes with non zero pending elements: %v\n%v", v, strings.Join(stageState, "")))
 					}
-					panic(fmt.Sprintf("nothing in progress and no refreshes with non zero pending elements: %v\n%v", v, strings.Join(stageState, "")))
+				} else {
+					nextEvent.Execute(em)
+					em.addPending(-1) // Decrement for the event being processed.
 				}
 			} else if len(em.inprogressBundles) == 0 {
 				v := em.livePending.Load()

Review Comment:
   I was wondering if we could refactor to reduce some of the nested if/else statements. I gave it a try only to validate for myself whether it might be done.
   ```
   defer em.refreshCmd.L.Unlock()
   if len(em.inprogressBundles) > 0 {
      return
   }
   if len(em.watermarkRefreshes) > 0 {
       v := em.livePending.Load()
       slog.Debug("Bundles: nothing in progress after advance",
   	    slog.Any("advanced", advanced),
   	    slog.Int("refreshCount", len(em.watermarkRefreshes)),
   	    slog.Int64("pendingElementCount", v),
       )
      return
   }
   
   nextEvent := em.testStreamHandler.NextEvent()
   if nextEvent != null {
       nextEvent.Execute(em)
       em.addPending(-1) // Decrement for the event being processed.
       return
   }
   
   v := em.livePending.Load()
   slog.Debug("Bundles: nothing in progress and no refreshes", slog.Int64("pendingElementCount", v))
   if v > 0 {
   var stageState []string
   ids := maps.Keys(em.stages)
   sort.Strings(ids)
   for _, id := range ids {
   	ss := em.stages[id]
   	inW := ss.InputWatermark()
   	outW := ss.OutputWatermark()
   	upPCol, upW := ss.UpstreamWatermark()
   	upS := em.pcolParents[upPCol]
   	stageState = append(stageState, fmt.Sprintln(id, "watermark in", inW, "out", outW, "upstream", upW, "from", upS, "pending", ss.pending, "byKey", ss.pendingByKeys, "inprogressKeys", ss.inprogressKeys, "byBundle", ss.inprogressKeysByBundle, "holds", ss.watermarkHoldHeap, "holdCounts", ss.watermarkHoldsCounts))
   }
   panic(fmt.Sprintf("nothing in progress and no refreshes with non zero pending elements: %v\n%v", v, strings.Join(stageState, "")))
   
   
   ```



##########
sdks/go/pkg/beam/runners/prism/internal/engine/engine_test.go:
##########
@@ -169,3 +169,50 @@ func TestElementManagerCoverage(t *testing.T) {
 		})
 	}
 }
+
+func TestTestStream(t *testing.T) {
+	initRunner(t)
+
+	tests := []struct {
+		pipeline func(s beam.Scope)
+	}{
+		{pipeline: primitives.TestStreamBoolSequence},
+		{pipeline: primitives.TestStreamByteSliceSequence},
+		{pipeline: primitives.TestStreamFloat64Sequence},
+		{pipeline: primitives.TestStreamInt64Sequence},
+		{pipeline: primitives.TestStreamInt16Sequence},
+		{pipeline: primitives.TestStreamStrings},
+		{pipeline: primitives.TestStreamTwoBoolSequences},
+		{pipeline: primitives.TestStreamTwoFloat64Sequences},
+		{pipeline: primitives.TestStreamTwoInt64Sequences},
+		{pipeline: primitives.TestStreamTwoUserTypeSequences},
+	}
+
+	configs := []struct {
+		name                              string
+		OneElementPerKey, OneKeyPerBundle bool
+	}{
+		{"Greedy", false, false},
+		{"AllElementsPerKey", false, true},
+		{"OneElementPerKey", true, false},
+		{"OneElementPerBundle", true, true},
+	}
+	for _, config := range configs {
+		for _, test := range tests {
+			t.Run(initTestName(test.pipeline)+"_"+config.name, func(t *testing.T) {
+				t.Cleanup(func() {
+					engine.OneElementPerKey = false
+					engine.OneKeyPerBundle = false
+				})
+				engine.OneElementPerKey = config.OneElementPerKey
+				engine.OneKeyPerBundle = config.OneKeyPerBundle
+				p, s := beam.NewPipelineWithRoot()
+				test.pipeline(s)
+				_, err := executeWithT(context.Background(), t, p)
+				if err != nil {
+					t.Fatalf("pipeline failed, but feature should be implemented in Prism: %v", err)
+				}
+			})
+		}
+	}
+}

Review Comment:
   I realize we have a `primitives/teststream_test.go` but was wondering if `TestTestStream` could be placed in a `enginge/teststream_test.go` file? 



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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1972000716

   Should be fixed by https://github.com/apache/beam/pull/30462.
   
   The log file was around half a million lines, so grep was a tiny bit unhappy.


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1939727181

   R: @damondouglas  (because it may help understand drain)
   cc: @jrmccluskey  (because it's test stream)


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1946708211

   Friendly Ping.


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on code in PR #30072:
URL: https://github.com/apache/beam/pull/30072#discussion_r1492852786


##########
sdks/go/pkg/beam/runners/prism/internal/engine/engine_test.go:
##########
@@ -169,3 +169,50 @@ func TestElementManagerCoverage(t *testing.T) {
 		})
 	}
 }
+
+func TestTestStream(t *testing.T) {
+	initRunner(t)
+
+	tests := []struct {
+		pipeline func(s beam.Scope)
+	}{
+		{pipeline: primitives.TestStreamBoolSequence},
+		{pipeline: primitives.TestStreamByteSliceSequence},
+		{pipeline: primitives.TestStreamFloat64Sequence},
+		{pipeline: primitives.TestStreamInt64Sequence},
+		{pipeline: primitives.TestStreamInt16Sequence},
+		{pipeline: primitives.TestStreamStrings},
+		{pipeline: primitives.TestStreamTwoBoolSequences},
+		{pipeline: primitives.TestStreamTwoFloat64Sequences},
+		{pipeline: primitives.TestStreamTwoInt64Sequences},
+		{pipeline: primitives.TestStreamTwoUserTypeSequences},
+	}
+
+	configs := []struct {
+		name                              string
+		OneElementPerKey, OneKeyPerBundle bool
+	}{
+		{"Greedy", false, false},
+		{"AllElementsPerKey", false, true},
+		{"OneElementPerKey", true, false},
+		{"OneElementPerBundle", true, true},
+	}
+	for _, config := range configs {
+		for _, test := range tests {
+			t.Run(initTestName(test.pipeline)+"_"+config.name, func(t *testing.T) {
+				t.Cleanup(func() {
+					engine.OneElementPerKey = false
+					engine.OneKeyPerBundle = false
+				})
+				engine.OneElementPerKey = config.OneElementPerKey
+				engine.OneKeyPerBundle = config.OneKeyPerBundle
+				p, s := beam.NewPipelineWithRoot()
+				test.pipeline(s)
+				_, err := executeWithT(context.Background(), t, p)
+				if err != nil {
+					t.Fatalf("pipeline failed, but feature should be implemented in Prism: %v", err)
+				}
+			})
+		}
+	}
+}

Review Comment:
   Not without making teststream_test.go be in `package engine_test`, which puts it outside of the engine package, so that we avoid a circular import loop by depending on test cases (from primitives), that depend on packages that depend on prism, enabling proper black box testing of the entire engine. (See the docs at the top of https://pkg.go.dev/testing for some details, and https://pkg.go.dev/cmd/go@master#hdr-Test_packages for others)
   
   I expect I'll want to actually add real unit tests once the processing time handling is in (never trust time), and those would need to be in `engine/teststream_test.go`.



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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1971863965

   Technically yes, but it also should have filtered out the tests. I'll take
   a look when I get back to my desk.
   
   On Thu, Feb 29, 2024, 10:51 AM Yi Hu ***@***.***> wrote:
   
   > Any chance this breaks Go Flink VR?
   >
   > From the history of first failed run,
   >
   >    - https://github.com/apache/beam/actions/runs/7936467225 (first
   >    failing)
   >    - https://github.com/apache/beam/actions/runs/7932752907 (last
   >    succeeded)
   >
   > image.png (view on web)
   > <https://github.com/apache/beam/assets/8010435/df410f59-685e-4c45-b9fd-4239ff9eef0d>
   >
   > this is the only Go change.
   >
   > However, the test log was huge and truncated, not able to investigate
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/beam/pull/30072#issuecomment-1971759400>, or
   > unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/ADKDOFJHMUHFGW2GHJ2GZG3YV54ELAVCNFSM6AAAAABCF6BMF6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZRG42TSNBQGA>
   > .
   > You are receiving this because you modified the open/close state.Message
   > ID: ***@***.***>
   >
   


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1971759400

   Any chance this breaks Go Flink VR?
   
   From the history of first failed run,
   - https://github.com/apache/beam/actions/runs/7936467225 (first failing)
   - https://github.com/apache/beam/actions/runs/7932752907 (last succeeded)
   
   <img width="552" alt="image" src="https://github.com/apache/beam/assets/8010435/df410f59-685e-4c45-b9fd-4239ff9eef0d">
   
   this is the only Go change.
   
   However, the test log was huge and truncated, not able to investigate
   


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1939728868

   Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control


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

Re: [PR] [#29917][prism] Initial TestStream support [beam]

Posted by "lostluck (via GitHub)" <gi...@apache.org>.
lostluck commented on PR #30072:
URL: https://github.com/apache/beam/pull/30072#issuecomment-1949163167

   PTAL! Thanks!


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