lhotari commented on pull request #10148:
URL: https://github.com/apache/pulsar/pull/10148#issuecomment-814616764
> This seems wrong, I already added this functionality, there is group
"extra" available to segregate tests, we can just change the one we wish to
exclude to that , they will not be run among the broker tests. We don't need
all these changes.
It might be wrong currently. I hope we get the changes eventually "right"
together.
The "extra" group didn't really exist. It was only mentioned in the `group`
parameter of a few `@BeforeMethod` annotations.
This isn't a sufficient solution. I learned by experimenting, that to
properly exclude a group in TestNG, the group has to be listed in the pom.xml's
default value for `excludedGroups`. This will resolve the issue with TestNG
where a single test method is added to another group:
for example:
```
@Test(groups = "flaky")
public class DeadLetterTopicTest extends ProducerConsumerBase {
...
@Test
public void testDeadLetterTopic() throws Exception {
....
}
...
@Test(groups = "quarantine")
public void testDeadLetterTopicByCustomTopicName() throws Exception {
...
}
...
...
}
```
In the above example, `testDeadLetterTopicByCustomTopicName` test case will
belong to groups `flaky` and `quarantine` at runtime ([explanation in TestNG
documentation](https://testng.org/doc/documentation-main.html#partial-groups)).
If there's no `excludedGroups` setting, this method will also get run for the
`flaky` group. This problem is resolved by adding `quarantine` to the
`excludedGroups`.
Since it's necessary to add the `quarantine` group to the `excludedGroups`,
that is the reason why it doesn't work as a solution to list all possible
groups in `@BeforeMethod` annotations.
For example, when `excludedGroups` contains `quarantine`, this
`BeforeMethod` doesn't get run at all, for any group:
```
@BeforeMethod(groups = { "broker", "websocket", "broker-api",
"broker-discovery", "broker-impl", "extra", "flaky", "quarantine" })
public void beforeMethod(Method m) throws Exception {
methodName = m.getName();
}
```
This problem is resolved by using
```
@BeforeMethod(alwaysRun = true)
public void beforeMethod(Method m) throws Exception {
methodName = m.getName();
}
```
I hope this explanation clarifies why this PR contains necessary changes if
we want to add a new TestNG group for quarantined tests. If our goal is
something else, this PR isn't necessary at all. In that case, we could simply
use TestNG's `org.testng.annotations.Ignore` annotation or
`@Test(enabled=false)`.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
