Please add "" like results="{{ remote_artifacts }}/results.yml"
remote_artifacts -- can have spaces

OK, will add. What about the role_result_failed variable in other roles?

Related to https://pagure.io/standard-test-roles/issue/296

Added quotes as suggested. @astepano, what about the role_result_failed variable? Are other roles working correctly if the variable is not used there?

@psss let's define next steps:

1. Take idea of https://pagure.io/standard-test-roles/pull-request/297#
2. Make sure that this scripts no matter what always return 0 (use trap to do some cleanups if need). <-- this is what we missing now
3. @ssahani made positional arguments, but we go with universal common approach by using options -s/--long. Like :

script: ../files/beakerlib-test.sh --workdir {{ tenv_workdir }} --arifacts {{ remote_artifacts }} --test {{ item }}

@psss : does this sound good for you?

@astepano, do you mean to wait until #297 is merged and then rebase the changes related to results.yaml on top of it?

297 is no finished.

The scripts can return exit code !=0
I propose to take #297 as some inspiration , and #297 close without merge.
We can discuss this is you want.

This is currently blocked by #326 and #327.

Rebased and updated to work with the latest changes in #326 and #335. Introduces the proposed error state for basic and beakerlib tests which results in a non-zero exit code of the ansible playbook. Please, review.

Example set of basic tests with corresponding results.yml:

> cat basic.yml
- hosts: localhost
  roles:
  - role: standard-test-basic
    tags:
    - classic
    tests:
    - good:
        dir: .
        run: /bin/true
    - bad:
        dir: .
        run: /bin/false
    - missing:
        dir: .
        run: /bin/missing
> cat /tmp/artifacts/results.yml
results:
- {result: pass, test: good}
- {result: fail, test: bad}
- {result: error, test: missing}

Example set of beakerlib tests with corresponding results.yml:

> cat beakerlib.yml
- hosts: localhost
  roles:
  - role: standard-test-beakerlib
    tags:
    - classic
    tests:
    - wizard/bad
    - wizard/good
    - wizard/problem
> cat /tmp/artifacts/results.yml
results:
- {result: fail, test: wizard/bad}
- {result: pass, test: wizard/good}
- {result: error, test: wizard/problem}

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

@psss to make it consistent, I think there is missing part for:

https://pagure.io/standard-test-roles/blob/master/f/roles/standard-test-beakerlib/files/run-beakerlib-test

Is it correct? Should run-beakerlib-test also be modified or in other PR? But this PR touches beakerlib role.

I am asking because, in case of basic role results.yaml is produced by script-runner, in case of beakerrole, it is produced by ansible code. For me this sounds as inconsistent. Can we move results reporting to run-beakerlib-test (creating the results.yaml file in run-beakerlib-test) ?

In #326 you have kept the part which is handling logs in the ansible playbook. I thought you had some intention why doing it in this way so I haven't changed your layout. So do you propose to completely move the always section into run-beakerlib-test?

@psss I am OK for any approach as long it is consistent. Same approach for basic and for beakerlib role.

OK, moved the log handling into run-beakerlib-test. Please, review.

Seems reasonable.
Merging, @psss thank you!

Commit 9d87c816 fixes this pull-request