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/26 20:59:21 UTC

[GitHub] [beam] fernando-wizeline opened a new pull request, #17471: [BEAM-14170] - Create a test that runs sickbayed tests

fernando-wizeline opened a new pull request, #17471:
URL: https://github.com/apache/beam/pull/17471

   Adding sickbayed/disabled tests to a new job 
   
   ------------------------
   
   Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [ x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
    - [ x] 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)
   ------------------------------------------------------------------------------------------------
   [![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)
   
   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] damccorm commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r882912467


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {
+  group = "Verification"
+  description "Validates Direct runner (Sickbay Tests)"
+  systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
+          "--runner=DirectRunner",
+  ])
+
+  classpath = configurations.needsRunner
+  testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs)
+
+  filter {
+    for (String test : sickbayTests) {
+      includeTestsMatching test
+    }
+
+    // https://issues.apache.org/jira/browse/BEAM-4191
+    includeTestsMatching 'org.apache.beam.runners.direct.WatermarkManagerTest.updateWatermarkWithDifferentWindowedValueInstances'

Review Comment:
   Hm, interesting - I don't immediately see anything wrong...
   
   Were you able to confirm that it does indeed run this test with this includeTestsMatching?
   
   I'm also curious if the exclusion works if you exclude `*. updateWatermarkWithDifferentWindowedValueInstances` - that would point to it being an issue with the exact string we're using.
   
   If neither of those point to the issue, I'm ok leaving it as is I guess



-- 
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] fernando-wizeline commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r882047968


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {

Review Comment:
   Hi Danny! I was creating the tasks programmatically, but since the tests for each runner are different, wouldn't it be better to leave the tasks like this instead of using conditional statements or creating a whole class hierarchy to dynamically access the tests for each runner?
   Sorry if this a silly question.



-- 
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] damccorm commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r882643729


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {
+  group = "Verification"
+  description "Validates Direct runner (Sickbay Tests)"
+  systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
+          "--runner=DirectRunner",
+  ])
+
+  classpath = configurations.needsRunner
+  testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs)
+
+  filter {
+    for (String test : sickbayTests) {
+      includeTestsMatching test
+    }
+
+    // https://issues.apache.org/jira/browse/BEAM-4191
+    includeTestsMatching 'org.apache.beam.runners.direct.WatermarkManagerTest.updateWatermarkWithDifferentWindowedValueInstances'

Review Comment:
   Whoops, accidentally deleted a comment, but I'm still unclear why this can't be included in sickbayTests



##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {

Review Comment:
   Yeah, actually taking another look I think you're right that this is more trouble than its worth, especially since we also need the tests in needsRunnerTests so there's not a super clean way to pull them out. Thanks for pushing on this. 



-- 
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] fernando-wizeline commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1132000381

   > @fernando-wizeline - feel free to ping if PR reviews are taking a while. Sometimes people miss notifications.
   
   Hi @aaltay! Is there someone who can help us to review this?
   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


[GitHub] [beam] fernando-wizeline commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1140292026

   > Thank you @damccorm
   > 
   > @fernando-wizeline - Could you ping me after all tests pass?
   
   Hi @aaltay! Looks like tests are all green now.


-- 
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] fernando-wizeline commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r882807538


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {
+  group = "Verification"
+  description "Validates Direct runner (Sickbay Tests)"
+  systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
+          "--runner=DirectRunner",
+  ])
+
+  classpath = configurations.needsRunner
+  testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs)
+
+  filter {
+    for (String test : sickbayTests) {
+      includeTestsMatching test
+    }
+
+    // https://issues.apache.org/jira/browse/BEAM-4191
+    includeTestsMatching 'org.apache.beam.runners.direct.WatermarkManagerTest.updateWatermarkWithDifferentWindowedValueInstances'

Review Comment:
   Oh, about this one: I'm using the same sickbayTests in both needsRunnerTests and validatesRunnerSickbay. For some reason, when I remove the Ignore tag from the updateWatermarkWithDifferentWindowedValueInstances method in class WatermarkManagerTest and rely solely on the excludeTestsMatching instruction, the test gets executed anyway. I'm not really knowledgeable in Gradle, so I'm pretty sure there's something I'm missing.
   The quickest workaround I came up with was to just leave the Ignore tag there and add the test directly to the validatesRunnerSickbay task.
   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


[GitHub] [beam] asf-ci commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1110242811

   Can one of the admins verify this patch?


-- 
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] fernando-wizeline commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r882927830


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {
+  group = "Verification"
+  description "Validates Direct runner (Sickbay Tests)"
+  systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
+          "--runner=DirectRunner",
+  ])
+
+  classpath = configurations.needsRunner
+  testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs)
+
+  filter {
+    for (String test : sickbayTests) {
+      includeTestsMatching test
+    }
+
+    // https://issues.apache.org/jira/browse/BEAM-4191
+    includeTestsMatching 'org.apache.beam.runners.direct.WatermarkManagerTest.updateWatermarkWithDifferentWindowedValueInstances'

Review Comment:
   Oh, yeah, I was able to run the validatesRunnerSickbay which in turn executed updateWatermarkWithDifferentWindowedValueInstances.
   I'll try to remove the ignore tag and exclude the test via `*. updateWatermarkWithDifferentWindowedValueInstances`
   Thanks again!



-- 
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] damccorm commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r878744381


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {

Review Comment:
   This task gets duplicated a lot, with the only differences being the runner and the classpath (and maybe the sickbayed test?). Instead of duplicating the code, could you just programatically create them like this? https://github.com/apache/beam/blob/e6c73b40f8376ec5088f69360108200ee5bd3813/examples/java/twitter/build.gradle#L81



##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {
+  group = "Verification"
+  description "Validates Direct runner (Sickbay Tests)"
+  systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
+          "--runner=DirectRunner",
+  ])
+
+  classpath = configurations.needsRunner
+  testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs)
+
+  filter {
+    for (String test : sickbayTests) {
+      includeTestsMatching test
+    }
+
+    // https://issues.apache.org/jira/browse/BEAM-4191
+    includeTestsMatching 'org.apache.beam.runners.direct.WatermarkManagerTest.updateWatermarkWithDifferentWindowedValueInstances'

Review Comment:
   Why do we just need this one on the direct runner and not the other ones?



-- 
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] aaltay commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
aaltay commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1140142960

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


[GitHub] [beam] damccorm commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
damccorm commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r878744124


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {
+  group = "Verification"
+  description "Validates Direct runner (Sickbay Tests)"
+  systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
+          "--runner=DirectRunner",
+  ])
+
+  classpath = configurations.needsRunner
+  testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs)
+
+  filter {
+    for (String test : sickbayTests) {
+      includeTestsMatching test
+    }
+
+    // https://issues.apache.org/jira/browse/BEAM-4191
+    includeTestsMatching 'org.apache.beam.runners.direct.WatermarkManagerTest.updateWatermarkWithDifferentWindowedValueInstances'

Review Comment:
   Why do we just need this one on the direct runner and not the other ones?



-- 
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] fernando-wizeline commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1110257802

   Hi @ibzib or @pabloem,
   Would you help me to review this?
   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


[GitHub] [beam] asf-ci commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1110242804

   Can one of the admins verify this patch?


-- 
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] fernando-wizeline commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r883116253


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {
+  group = "Verification"
+  description "Validates Direct runner (Sickbay Tests)"
+  systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
+          "--runner=DirectRunner",
+  ])
+
+  classpath = configurations.needsRunner
+  testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs)
+
+  filter {
+    for (String test : sickbayTests) {
+      includeTestsMatching test
+    }
+
+    // https://issues.apache.org/jira/browse/BEAM-4191
+    includeTestsMatching 'org.apache.beam.runners.direct.WatermarkManagerTest.updateWatermarkWithDifferentWindowedValueInstances'

Review Comment:
   Looks like adding it using `*. updateWatermarkWithDifferentWindowedValueInstances` didn't do the trick. I'm going to revert the change.



-- 
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] aaltay commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
aaltay commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1133514498

   R: @damccorm - Would you be able to do a first pass review on this change?


-- 
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] tvalentyn commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1158003695

   I am seeing Samza runner tests have been [perma-failing ](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/) starting from this run: 
   
   https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/9551/
   
   This PR was the only change that was added between passing and failing runs. Any chance that Samza VR suite became misconfigured and we include flaky tests in the regular VR suite?


-- 
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] asf-ci commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1110242809

   Can one of the admins verify this patch?


-- 
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] fernando-wizeline commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1110256244

   Hi @kennknowles!
   Can you please run a seed job to enable the new job?
   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


[GitHub] [beam] asf-ci commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1110242806

   Can one of the admins verify this patch?


-- 
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] aaltay commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
aaltay commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1125444970

   @fernando-wizeline - feel free to ping if PR reviews are taking a while. Sometimes people miss notifications.


-- 
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] aaltay merged pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
aaltay merged PR #17471:
URL: https://github.com/apache/beam/pull/17471


-- 
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] aaltay commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
aaltay commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1140143006

   Thank you @damccorm 
   
   @fernando-wizeline - Could you ping me after all tests pass?


-- 
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] fernando-wizeline commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r882807538


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {
+  group = "Verification"
+  description "Validates Direct runner (Sickbay Tests)"
+  systemProperty "beamTestPipelineOptions", JsonOutput.toJson([
+          "--runner=DirectRunner",
+  ])
+
+  classpath = configurations.needsRunner
+  testClassesDirs = files(project(":sdks:java:core").sourceSets.test.output.classesDirs)
+
+  filter {
+    for (String test : sickbayTests) {
+      includeTestsMatching test
+    }
+
+    // https://issues.apache.org/jira/browse/BEAM-4191
+    includeTestsMatching 'org.apache.beam.runners.direct.WatermarkManagerTest.updateWatermarkWithDifferentWindowedValueInstances'

Review Comment:
   Oh, about this one: I'm using the same sickbayTests in both needsRunnerTests and validatesRunnerSickbay. For some reason, when I remove the @Ignore tag from the updateWatermarkWithDifferentWindowedValueInstances method in class WatermarkManagerTest and rely solely on the excludeTestsMatching instruction, the test gets executed anyway. I'm not really knowledgeable in Gradle, so I'm pretty sure there's something I'm missing.
   The quickest workaround I came up with was to just leave the @Ignore tag there and add the test directly to the validatesRunnerSickbay task.
   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


[GitHub] [beam] fernando-wizeline commented on a diff in pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on code in PR #17471:
URL: https://github.com/apache/beam/pull/17471#discussion_r880744078


##########
runners/direct-java/build.gradle:
##########
@@ -178,6 +192,26 @@ task validatesRunner(type: Test) {
   }
 }
 
+tasks.register("validatesRunnerSickbay", Test) {

Review Comment:
   thanks for the review Danny! Will follow the suggestions and let you know once done.



-- 
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] fernando-wizeline commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
fernando-wizeline commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1139720510

   > I'm good taking this as is. I'm curious about the updateWatermarkWithDifferentWindowedValueInstances issue, but I don't think its worth blocking on. Thanks for sticking with this!
   
   Thanks to you Danny!


-- 
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] damccorm commented on pull request #17471: [BEAM-14170] - Create a test that runs sickbayed tests

Posted by GitBox <gi...@apache.org>.
damccorm commented on PR #17471:
URL: https://github.com/apache/beam/pull/17471#issuecomment-1158006033

   I think https://github.com/apache/beam/pull/21916 is meant to address this


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