Additional test coverage about acceptance-test-harness HOT 12 OPEN

It started with #556

OT: I have been using containerized web driver since long before this change :-/

firefox-container and chrome-container would appear to both be broken - with no reports of that (to the point I was going to rip out that code as unused :-o )

Why? There are so many areas in Jenkins core that are not UI tested.

there are also so many parts of plugin UIs that are not tested. I am not saying we should not add more testing of Jenkins core - but that any added testing should not be exclusively for Jenkins core.

And for instance, the new Ant tests add new integration tests for Ant here in the ATH, these should be part of the Ant plugin, not ATH. (They even can be implemented as integration tests without UI layer).

tests should be in the plugins and then something similar to PCT should run the tests / a selection of them from here.
That way tests in the plugins pickup on regressions and aren't run in isolation and tests here pickup on core issues.

whilst exercising the UI done in a unit test using HTML Unit it is in my experience more brittle and costly to maintain.
Additionally HTML Unit is not a real browser and behaves differently in several situations.

We could use what you have in warnings-ng to run those ATH tests - but this adds another issues/question.

1. We would require more costly build resources (memory) - and as the PCT is generic if we ran UI tests in the PCT that would need to scale across many/all of the plugins as we do not want to care about specifics of plugins - I believe this would add a cost to the project that without extra investment to rework a large portion would not be palatable. Currently I do not know how costly the PCT is to run for the Jenkins organisation - but with ~100+ plugins this multiples the costs quite a bit.

2. Additionally the PCT would need to upgrade the dependency to a version of the ATH that was compatible with the version of Jenkins being tested.

3. we need a way to run plugin tests on core PR changes. Unfortunate the number of ATHs tests we run against core in PRs is so limited today we have alas slipped regressions that where picked up by full runs of ATHs at a later date.

4. the page objects for plugins need to be shared - and as such that also now brings in a matrix of dependencies. the ATH installs plugins from the UpdateCenter (whatever is the latest) and so you now also need to potentially bring in those changes which means a lot of overhead in co-ordination/

5. the ATH framework only needs to be compatable with the current LTS version of Jenkins and its weekly, If a plugin upgrades to a new LTS then its page objects may have to adapt. this is fine - but then if your plugin depends on $otherplugin then you will pull in a version of the old page objects (which is fine) - but then how would a PCT work... your page objects use the older code - the other plugin now needs to use the newer code - the ATH that is used will not support the version of Jenkins that your plugin compiled against if it is older than the LTS - causing every plugin that uses ATHs to have to bump to the latest LTS when it is released if there are any changes to page layout (which happen regularly at the moment with the UI rework that is ongoing).

In summary I do not believe we are in a position today to have UI tests in plugins code to pick up on regressions. Until we are I believe that putting UI tests into plugin code this the wrong path. I am not objecting to anyone wanting to do the work to make this happen, but until we have it in place I think blocking any submissions using "should be in plugins repo" is wrong.

from acceptance-test-harness.

Did you already talk with the authors of the plugins that you are adding new tests for? Are they willing to take over the maintenance of these new tests here in the ATH? One of the reasons that Oliver and I started the transition of tests to individual plugins was the problem that breaking tests were never noticed by the plugin authors here and so the ATH degenerated. Currently we are are green 🎉 , but this requires that someone actively maintains these tests.

from acceptance-test-harness.

In summary I do not believe we are in a position today to have UI tests in plugins code to pick up on regressions. Until we are I believe that putting UI tests into plugin code this the wrong path. I am not objecting to anyone wanting to do the work to make this happen, but until we have it in place I think blocking any submissions using "should be in plugins repo" is wrong.

I am sorry, but this already is in place for years. IIRC this has happened before you stepped in, and it permitted us to move away plugin suites with to many tests and too few installs that ware simply not worth keeping up-to-date with plugin codebase by ATH maintainers (putting aside that all regressions were found post-release anyway).

• nor would dependabot offer any update to Jenkins in a plugin (and even then if we had weekly PRs to bump Jenkins (and monthly to test an LTS) in all plugins and run there UI tests - I can imaging the infra team response :) )- so tests stored in a plugin would not detect regressions (at least in core) for potentially a very long time.

I am using actively dependabot to upgrade the Jenkins version in my plugin UI tests. This does not detect broken Jenkins PRs, but at least it detects broken Jenkins releases one week later.

Example: jenkinsci/warnings-ng-plugin#1550

• tests in plugins cause the plugin to be rebuilt against the new core / plugins - so if there is a binary incompatibility (but not a source one) then it is not detected in the "UI test - it is also testing against what is in master which may not be the release of the plugin. (or more likely a source incompatibility but not a binary one - which would outright fail the build even though UI tests would be unaffected by the release binary)

This is not the way it works currently. The Jenkins version for UI tests is different from the Jenkins version for the build. So it will detect binary incompatibilities with newer Jenkins versions.

from acceptance-test-harness.

I would recommend to move those tests to the corresponding plugins and not to the ATH. At least that was the consensus that we had quite a couple of years ago. It would only make sense to add more tests here that actually test core functionality that is not tested here yet. Otherwise we are bloating up the ATH again. @olivergondza introduced a successful concept for UI tests in plugins that should work here as well. I am also using this concept in all of my plugins very successfully.

from acceptance-test-harness.

cc @jtnord @rsandell @olamy @ampuscas ^^

from acceptance-test-harness.

I would recommend to move those tests to the corresponding plugins and not to the ATH. At least that was the consensus that we had quite a couple of years ago

not to my knowledge or recollection. In fact I would say the ATH tests for the warnings-ng are only in the warnings-ng repo is an anti-pattern as they are not picking up regressions caused by changes in Jenkins in a reasonably timely fashion (they are not run as part of the PCT).

Having some UI tests in the plugin code is reasonable a it allows the testing for regressions in plugin code when developing the plugin - but there also needs to be some coverage in the ATH.

Additionally and this is a not the current state - the ATH tests here should really be exercising human flow now individual plugins - however that ship has sailed and is a larger effort that can be done in the future.

It would only make sense to add more tests here that actually test core functionality that is not tested here yet.

core to what Jenkins or plugins?

if Jenkins then I disagree strongly.

for plugins, Yes we should not be duplicating existing tests as I wrote "the tests do not significantly overlap."

from acceptance-test-harness.

I would recommend to move those tests to the corresponding plugins and not to the ATH. At least that was the consensus that we had quite a couple of years ago

not to my knowledge or recollection.

It started with #556

In fact I would say the ATH tests for the warnings-ng are only in the warnings-ng repo is an anti-pattern as they are not picking up regressions caused by changes in Jenkins in a reasonably timely fashion (they are not run as part of the PCT).
Having some UI tests in the plugin code is reasonable a it allows the testing for regressions in plugin code when developing the plugin - but there also needs to be some coverage in the ATH.

Yes, I think a single test per important plugin would be helpful.

Additionally and this is a not the current state - the ATH tests here should really be exercising human flow now individual plugins - however that ship has sailed and is a larger effort that can be done in the future.

It would only make sense to add more tests here that actually test core functionality that is not tested here yet.

core to what Jenkins or plugins?

if Jenkins then I disagree strongly.

Why? There are so many areas in Jenkins core that are not UI tested.

And for instance, the new Ant tests add new integration tests for Ant here in the ATH, these should be part of the Ant plugin, not ATH. (They even can be implemented as integration tests without UI layer).

from acceptance-test-harness.

FWIW I think the tests should be in the plugins and then something similar to PCT should run the tests / a selection of them from here.

That way tests in the plugins pickup on regressions and aren't run in isolation and tests here pickup on core issues.

from acceptance-test-harness.

FWIW I think the tests should be in the plugins and then something similar to PCT should run the tests / a selection of them from here.

That way tests in the plugins pickup on regressions and aren't run in isolation and tests here pickup on core issues.

Yes, that would be the best approach. As far as I understand the PCT, all tests of each participating plugin are executed (even the unit and architecture tests), that makes not really sense. Maybe we should allow plugins to mark tests which get executed in PCT. These tests should include integration tests and UI tests.

from acceptance-test-harness.

There was always this dilemma coined by @uhafner and @jtnord: We want those tests maintained by component (plugin/core) owners, but they would not do it when tests are hosted in this repo. If we move all the tests to those repos, then ATH becomes toothless in verifying LTS, plugin sets and likely even the cloudbees offering mentioned. We also want ATH to be rich in providing solid grantees, but we want it executed fast and have low maintenance cost at the same time.

So we put together guidelines of what tests should go to ATH, and what should go to the plugin repo: https://github.com/jenkinsci/acceptance-test-harness/blob/master/docs/CONTRIBUTING.md#test-contribution. And as Ulli have said, we have a way to have ATH based tests in plugin core (for plugins that are not popular enough, or that are using ATH extensively enough so - if hosted inside ATH repo - it would cause ATH execution to slow down (beyond what is reasonable from community perspective)).

I lost touch with ATH long time ago, though I still see this distinction to be a valid one. Maybe the list of use-cases defined thereof needs updating, and as a result the guidelines too - I cannot tell.

@jtnord, albeit no longer an ATH contributor - I only use ATH as lib in plugin tests, I support the donation of the tests proposed by you (Thanks!). Although I advise this to be done only where this is in accordance with the guidelines.

from acceptance-test-harness.

In summary I do not believe we are in a position today to have UI tests in plugins code to pick up on regressions. Until we are I believe that putting UI tests into plugin code this the wrong path. I am not objecting to anyone wanting to do the work to make this happen, but until we have it in place I think blocking any submissions using "should be in plugins repo" is wrong.

I am sorry, but this already is in place for years. IIRC this has happened before you stepped in, and it permitted us to move away plugin suites with to many tests and too few installs that ware simply not worth keeping up-to-date with plugin codebase by ATH maintainers (putting aside that all regressions were found post-release anyway).

Some of the examples - except for those mentioned by @uhafner:

• https://github.com/jenkinsci/kerberos-sso-plugin/tree/master/ui-tests
• https://github.com/jenkinsci/openstack-cloud-plugin/tree/master/ui-tests

firefox-container and chrome-container would appear to both be broken - with no reports of that (to the point I was going to rip out that code as unused :-o )

Please, don't :)

from acceptance-test-harness.

In summary I do not believe we are in a position today to have UI tests in plugins code to pick up on regressions. Until we are I believe that putting UI tests into plugin code this the wrong path. I am not objecting to anyone wanting to do the work to make this happen, but until we have it in place I think blocking any submissions using "should be in plugins repo" is wrong.

I am sorry, but this already is in place for years. IIRC this has happened before you stepped in, and it permitted us to move away plugin suites with to many tests and too few installs that ware simply not worth keeping up-to-date with plugin codebase by ATH maintainers (putting aside that all regressions were found post-release anyway).

• The current code requires host based networking for docker this can be removed we can say it is ready for general consumption as you are not locking out a set of developers from running the tests. WSL2 does not help here.
• The PCT does not run UI tests that are stored in a different module, nor would dependabot offer any update to Jenkins in a plugin (and even then if we had weekly PRs to bump Jenkins (and monthly to test an LTS) in all plugins and run there UI tests - I can imaging the infra team response :) )- so tests stored in a plugin would not detect regressions (at least in core) for potentially a very long time.
• tests in plugins cause the plugin to be rebuilt against the new core / plugins - so if there is a binary incompatibility (but not a source one) then it is not detected in the "UItest - it is also testing against what is inmaster` which may not be the release of the plugin. (or more likely a source incompatibility but not a binary one - which would outright fail the build even though UI tests would be unaffected by the release binary)

Randomly aside there can always be a 3rd way - the same tests can be in here and in a plugin - so they are excursive by the plugin (so they are maintained) and they are run on plugin development - we just need to publish the test-jar and then get the plugin to depend on the test jar and run tests from here - with some surefire config to only run tests that it wants via include patterns or via adding categories to the tests where a category is a plugin involved. (or any other way). Main issue could be that we only strive to maintain compatability with the current LTS and weekly - and plugins may be using something older - as well as only a few older versions of plugins - but maybe that is enough (we don;t actually aggressivly cleanup old LTS support right now - there is some things for TODO 1.4xx in the code!) Basil has been investigating Launchable which can help with PR time (only running tests affected by the change) which can help alleviate the concern of test runtime. I am now digressing...

firefox-container and chrome-container would appear to both be broken - with no reports of that (to the point I was going to rip out that code as unused :-o )

Please, don't :)

Now I know it is used I won't remove it without a tested replacement.

However I have tried numerous times to reproduce / fix #1090 but have been unable to reproduce it - so am unable to debug or fix it. I still think there is some value to be had in removing it and using test containers to start and inject a remote-webdriver-url and this would fix the issue of host-based networking as well - but replacing it is no where near a priority for me :)

from acceptance-test-harness.