[CLOUD-3025] Allow selected EAP CD 14 / EAP 7.2.0.CR1 tests with updated JGroups

[iankko]

to run also for RH-SSO 7.3 TP CD and RH-SSO 7.3.0.ER1 images

Also update the warning message for some JGroups tests to the form expected
by the jboss-eap-config-jgroups module, namely:

• https://github.com/jboss-container-images/jboss-eap-modules/blob/master/jboss-eap-config-jgroups/added/launch/jgroups.sh#L145 and
• https://github.com/jboss-container-images/jboss-eap-modules/blob/master/jboss-eap-config-jgroups/added/launch/jgroups.sh#L152

and enable these tests too (drop the @ignore label to enable them for EAP CD,
EAP 7.2.0, RH-SSO 7.3 TP CD & RH-SSO 7.3.0.ER1 images

Signed-off-by: Jan Lieskovsky jlieskov@redhat.com

[iankko]

@luck3y @bstansberry PTAL (once got a chance)

Thank you, Jan

@douglaspalmer jFY, -^

[luck3y]

@iankko Sorry for the delay in looking at this. I think in the long term it would be better if we had a @common annotation for these tests and then cekit could also run that one as part of your image tests, otherwise you'll be opening PRs against an EAP repo for SSO tests, which isn't ideal IMO.

WDYT?

[bstansberry]

These tests are written and maintained for EAP. I don't mind too much adding some tags to avoid people having to duplicate tests, but in the end the tests are for EAP and are maintained as such. So if we need to change something, we change something, and if that's a problem for other testsuites then the tests should be duplicated.

Obviously we don't want to be changing tests all the time but when we need to we need to.

[luck3y]

@iankko theres one other thing you may want to consider too, which we found to be annoying in EAP CD. If you have the default product name container -tech-preview, then you have to duplicate the annotations, but thats supposed to be added from either an override.yaml or using cekit --build-test-preview, which should then adjust and run the expected tests.

[douglaspalmer]

That isn’t true Brian, many of these test were common tests shared between EAP and the images built on top of EAP.

…

On Nov 27, 2018, at 4:08 PM, Brian Stansberry ***@***.***> wrote: These tests are written and maintained for EAP. I don't mind too much adding some tags to avoid people having to duplicate tests, but in the end the tests are for EAP and are maintained as such. So if we need to change something, we change something, and if that's a problem for other testsuites then the tests should be duplicated. Obviously we don't want to be changing tests all the time but when we need to we need to. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAnM8UTfNZTywTucNQebfLNDtfZNDSj-ks5uzdQbgaJpZM4Ywwyj>.

[luck3y]

@douglaspalmer I think what Brian means is that now that we've split up the products & tests coordination of this stuff is going to be much more difficult, and if it were the case that we needed a test for EAP changed, we wouldn't necessarily know it would break anyone else using it until they would tell us. That would result in a PR to update the test, or add a new one, and that isn't really any more manageable than just having all of the tests in a shared repo, which none of us have commit access to. The counter-argument is that its some additional duplication of tests.

Perhaps a comprimise, until we get something like a @common annotation might be that we'll accept PRs against common, and if people want they can always duplicate those tests in their own suites anyway, there aren't really all that many of them are there?

[luck3y]

Yeah, so looking at the patch again, if we rework the "basic" tests to be something like "shared" tests and just modify those, I think that could be ok, what do you guys think?

[luck3y]

Also, one last thought, if perhaps this was caused by me moving too many tests out of cct_module, then we can rectify that by just adding those basic tests back to cct_module IMO.

[iankko]

@bstansberry @luck3y

The goal here basically is to allow other images to run those tests too (without needing to first tag the image with expected tag). Maybe retagging the tests with @common tag would solve this (this PR was created yet before the discussion how to handle this started, so original approach might not the be right one).

I don't mind / have objections, the EAP team to change something in the test, when there's necessity for it (of course having the ability to run the tests without the duplication allow us to catch possible issues too, so you wouldn't be left alone to keep the tests fixed / updated). For us, it would have the benefit we wouldn't need to duplicate / copy all of the tests in our own repo (we could dupe only those, that would differ from EAP form [if any]).

Does that sound reasonable?

Thank you, Jan

[bstansberry]

To clarify, I don't have any objection to this PR being merged as is as a short term solution, assuming the details are ok with @luck3y, e.g. the previously ignored tests.

I think we need to think a bit about some sort of better organizational principle though, vs adhoc annotating of tests, which could get messy. TBH I'm not sure what that should be, and maybe this is fine. We're going to need per sprint tags of this repo, so if consumers of this module are tied to a tag they are going to be running that tag's tests for their images. If master moves on and tests change to match current behavior, that's only relevant for images that are consuming master.

FWIW, this basic.feature file doesn't seem terribly "basic", e.g. there are all these tests that were ignored and now are not that are validating small details of JGroups behavior. That's another form of organization that's worth improving.

[luck3y]

@bstansberry The ignored tests I consider a bug, they should certainly be unignored at this point ...

[douglaspalmer]

I’ve long believed that we have the granularity wrong for the images. We should build, configure and test at the subsystem level. With each product being composed from the available subsystems.

…

On Nov 28, 2018, at 8:05 AM, Brian Stansberry ***@***.***> wrote: To clarify, I don't have any objection to this PR being merged as is as a short term solution, assuming the details are ok with @luck3y <https://github.com/luck3y>, e.g. the previously ignored tests. I think we need to think a bit about some sort of better organizational principle though, vs adhoc annotating of tests, which could get messy. TBH I'm not sure what that should be, and maybe this is fine. We're going to need per sprint tags of this repo, so if consumers of this module are tied to a tag they are going to be running that tag's tests for their images. If master moves on and tests change to match current behavior, that's only relevant for images that are consuming master. FWIW, this basic.feature file doesn't seem terribly "basic", e.g. there are all these tests that were ignored and now are not that are validating small details of JGroups behavior. That's another form of organization that's worth improving. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAnM8WJL1pIIo1Nsk9eHJS1Po9MxmmPjks5uzrQsgaJpZM4Ywwyj>.

[bstansberry]

@iankko I merged this but haven't touched the JIRA as it includes more work than this.

[iankko]

@bstansberry Sure, no worries (I will handle that). Thank you for merging this!