PR#155: Fix to match requirements of STI spec (tests/tests*.yml) - fedora-zuul-jobs-config

fedora-zuul-jobs-config

#155 Fix to match requirements of STI spec (tests/tests*.yml)

Merged 2 years ago by zuul. Opened 2 years ago by fbo.

fbo/fedora-zuul-jobs-config sti-spec-fix into master

Remove useless use of find and rely on return code

Fabien Boucher • 2 years ago

b5062dc

Fix to match requirements of STI spec (tests/tests*.yml)

Fabien Boucher • 2 years ago

b9eddcd

playbooks/rpm/check-for-sti-tests.yaml

file modified

+7 -5

		`@@ -1,18 +1,20 @@`
		`---`
		`- hosts: localhost`
		`tasks:`
		`- - name: Check for tests/tests.yml exists`
		`- stat:`
		`- path: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/tests/tests.yml"`
		`+ - name: Check for tests/tests*.yml exists`
		`+ shell: "ls tests/tests*.yml"`
		`+ failed_when: false`
		`register: tests_stat`
		`+ args:`
		`+ chdir: "{{ zuul.project.src_dir }}"`
		`- zuul_return:`
		`data:`
		`zuul:`
		`child_jobs:`
		`- rpm-sti-test`
		`- when: tests_stat.stat.exists`
		`+ when: tests_stat.rc == 0`
		`- zuul_return:`
		`data:`
		`zuul:`
		`child_jobs: []`
		`- when: not tests_stat.stat.exists`
		`+ when: not tests_stat.rc == 0`

playbooks/rpm/check-for-tests.yaml

file modified

+7 -5

		`@@ -1,18 +1,20 @@`
		`---`
		`- hosts: localhost`
		`tasks:`
		`- - name: Check for tests/tests.yml exists`
		`- stat:`
		`- path: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/tests/tests.yml"`
		`+ - name: Check for tests/tests*.yml exists`
		`+ shell: "ls tests/tests*.yml"`
		`+ failed_when: false`
		`register: tests_stat`
		`+ args:`
		`+ chdir: "{{ zuul.project.src_dir }}"`
		`- zuul_return:`
		`data:`
		`zuul:`
		`child_jobs:`
		`- rpm-test`
		`- when: tests_stat.stat.exists`
		`+ when: tests_stat.rc == 0`
		`- zuul_return:`
		`data:`
		`zuul:`
		`child_jobs: []`
		`- when: not tests_stat.stat.exists`
		`+ when: not tests_stat.rc == 0`

fbo commented 2 years ago

This patch fixes an issue where only tests/tests.yml was checked
for embeded STI tests.

This also removes the useless usage of ansible_user_dir.

zuul commented 2 years ago

Build succeeded.

linters : SUCCESS in 1m 49s

churchyard commented on line 9 of playbooks/rpm/check-for-sti-tests.yaml 2 years ago

The * is expanded by shell, so using find gives no added benefit. The same result is returned with ls or stat.

churchyard commented on line 20 of playbooks/rpm/check-for-sti-tests.yaml 2 years ago

Should this check the exit code rather than output?

churchyard commented 2 years ago

Either way, the code looks like it'll work, but I don't know how to verify.

fbo commented 2 years ago

We are in a Zuul config repository so unfortunately this cannot be tested speculatively with "Depends-on". I did a test locally by running the play with ansible-playbook and an inventory file to set zuul.project.src_dir var and replacing the zuul_return module with debug module. This shows that it seems to behave as expected.

So IMO, let's merge and see if we get any trouble. Is that fine for you ?

fbo commented 2 years ago

Just saw you inline comment. Ok, wait I'll see to address them.

1 new commit added