DAOS-19247 test: Support scripted Test stages

[phender]

Adding the scriptedTestRpmStage() method to provide a scripted stage definition for the Test RPM
stages and the scriptedUnitTestStage() method to provide a scripted stage definition for the Fault
Injection Testing stage. Fixing skipping Functional test stages when builds are re-run and the stage
previously passed.

[phender]

Testing new functionality in daos-stack/daos#18597

[phender]

Downstream testing skipped as it will not test the new groovy code. New code verified as part of daos-stack/daos#18597

[grom72]

Suggested change

	* unitTestArgs Map of arguments to pass to unitTest; defaults to an empty Map.
	* unitTestPostArgs Map of arguments to pass to unitTestPost() for the stage
	* archiveArtifactsArgs Map of arguments to pass to archiveArtifacts() for the stage
	* unitTestArgs Map of arguments to pass to unitTest; defaults to an empty Map.
	* unitTestPostArgs Map of arguments to pass to unitTestPost() for the stage; defaults to an empty Map.
	* archiveArtifactsArgs Map of arguments to pass to archiveArtifacts() for the stage; defaults to an empty Map.

[phender]

Fixed in 8130ede

[grom72]

Why do we add ' daos-client-test'?
The actual Jenkins file does not require it:

                stage('Unit Test') {
...
                    steps {
                            job_step_update(
                                unitTest(timeout_time: 60,
                                        unstash_opt: true,
                                        inst_repos: daosRepos(),
                                        inst_rpms: unitPackages(target: 'el9'),
                                        image_version: 'el9.7',
                                        )
                            )
                    }
...
                }

Suggested change

	if (!unitTestArgs.containsKey('inst_rpms')) {
	/* groovylint-disable-next-line DuplicateStringLiteral */
	unitTestArgs['inst_rpms'] = unitPackages(target: distro) + ' daos-client-tests'
	}
	if (!unitTestArgs.containsKey('inst_rpms')) {
	/* groovylint-disable-next-line DuplicateStringLiteral */
	unitTestArgs['inst_rpms'] = unitPackages(target: distro)
	}

[phender]

This is needed for the Fault Injection Testing stage, where scriptedUnitTestStage() is being used.

[grom72]

The problem I see here (and in previous comments) is that we start using the scriptedUnitTestStage() method not for default unit test behavior but for an exceptional.
If Fault Injection requires some parameters different than default ones (e.g. additional daos-client-tests) they should be provided in Jenkinsfile (or via different wrapper function).

[daltonbohning]

I do agree with @grom72 here. IMO it's better to not hardcode this kind of thing into pipeline-lib

[phender]

The tricky part is that methods like unitPackages() rely on parseStageInfo() and parseStageInfo relies on the environment variables like the stage name being set. Since this method returns the stage definition to be run, the stage name is not yet set to the stage we're going to run, so the return value of unitPackages() cannot be passed as an input - in its current form. We would essentially need to replace all the parseStageInfo() outputs with required inputs into a bunch of groovy methods. Then we could call:

scriptedUnitTestStage(
   ...
   inst_rpms: unitPackages(target: distro, script: 'ci/unit/required_packages.sh') + ' daos-client-tests',
   ....
)

This is something I think is better tackled in another PR - one where we could also consider using scriptedUnitTestStage() for the other stages that use unitTest().

[phender]

The removal of the env.UNIT_TEST quick exit in vars/testsInStage.groovy was verified not to break the Commit Pragma tests in https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/pipeline-lib/job/PR-526/35/stages with a Jenkinsfile self-reference. With this removed the master PR testing these pipeline-lib changes was able to execute the expected code paths.

[daltonbohning]

Why empty string instead of null?

[daltonbohning]

Is this not the same?

Suggested change

	Map jobStatus = kwargs.get('jobStatus', null) ?: [:]
	Map jobStatus = kwargs.get('jobStatus', [:])

[phender]

Not exactly:

def kwargs = [jobStatus: false]

// #1 Returns [:]    (same result for null, 0, or '' jobStatus value or an missing jobStatus key)
kwargs.get('jobStatus', null) ?: [:]

// #2 Returns false  ([:] is only returned if the 'jobStatus' key is not assigned in kwargs, regardless of its value)
kwargs.get('jobStatus', [:])

[daltonbohning]

Why would we have [jobStatus: false] if it is supposed to be a map? This seems like we are masking other issues?