Removing SAMLPluginTest

[jglick]

As of jenkinsci/saml-plugin#93 in 2.0.1, the SAML plugin test here are redundant—basically the same coverage, just slower and harder to run, and apparently ignored since jenkinsci/saml-plugin#90 introduced a regression which did not block the plugin release and was not communicated to the plugin developer after the fact either.

[jglick]

@kuisathaverat any opinion?

[kuisathaverat]

Go a head, the same test is running on the Plugin

[jtnord]

The test in the plugin is for a different audience. The ATH is used to ensure no regressions when releasing a security or LTS version. Plugins tests at not re-run at this point.

Unclear why the test failure was not communicated, possibly due to the current sea of red, possibly also because no one is looking at the master job output anymore rather just running the ATH locally?

[jglick]

no regressions when releasing a security or LTS version. Plugins tests at not re-run at this point.

Currently true, at least in OSS; CloudBees would run plugin tests against new LTS versions via PCT, at least in the case of this plugin. Maybe the LTS checklist can be extended to include PCT? Someone would need to set up a job for it, and decide which plugins get covered.

possibly due to the current sea of red, possibly also because no one is looking at the master job output anymore

Yeah I suspect no one is paying much attention to the numerous known failures, much less flakes. (Other than SmokeTests, run by the jenkinsci/jenkins build.)

[timja]

Currently true, at least in OSS; CloudBees would run plugin tests against new LTS versions via PCT, at least in the case of this plugin. Maybe the LTS checklist can be extended to include PCT? Someone would need to set up a job for it, and decide which plugins get covered.

Bom is done for new lines at least currently

[timja]

Yeah I suspect no one is paying much attention to the numerous known failures, much less flakes. (Other than SmokeTests, run by the jenkinsci/jenkins build.)

No there's too many failures

[kuisathaverat]

I personally forget to run the PCT many times. I really don’t like it, every time I have to run it my scripts did not run and it is a pain to review the changes and run it. For that reason I preferred to have all the integration tests on the plugins and run them on each iteration rather than run the PCT.
Other option is make the PCT run on merges to the main branch and send the proper notifications, but I dunno if it is factible on Jenkins.io

[jglick]

Bom is done for new lines at least currently

Right, but this only matters for plugins in the bom, which saml is not.

I preferred to have all the integration tests on the plugins and run them on each iteration rather than run the PCT.

Are you talking about PCT or ATH?

To be clear, the regression I found was caused by a saml plugin change and had nothing to do with the version of Jenkins core or other plugins. Thus, it would have been caught by the PR build had the test I created been present before that change.

IMO the ATH is useful on occasion for verifying true GUI behaviors. The test being deleted here does not really match that description.

[amuniz]

We have an internal test for CloudBees SSO using SAML which relies on the page objects and docker fixture being removed here. Could we keep them?

No objection on removing the test itself.

[jglick]

Could we keep them?

Why not just copy those classes to your internal tests if you are using them?

[amuniz]

Ok. Fine.

[jglick]

an internal test for CloudBees SSO using SAML which relies on the page objects and docker fixture being removed here

Resolved, FTR