#327 Move basic role test execution to a separate file
Closed by astepano. Opened by psss.
psss/standard-test-roles run-basic-test  into  master

Download 327.patch

Adds a new script run-basic-test which contains test execution
logic for the standard-test-basic role. Uses environment variables
to hand over necessary parameters. Fails when required parameters
are not provided or test command is not found.

Simple playbook for testing and resulting logs:

- 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

/tmp/artifacts/FAIL_bad.log

Test name: bad
Test directory: /var/str/.
Test command: /bin/false
Test bad finished with exit code 1.

/tmp/artifacts/missing.log

Test name: missing
Test directory: /var/str/.
Test command: /bin/missing
/root/.ansible/tmp/ansible-tmp-1554735263.124157-250269934772953/run-basic-test: line 59: /bin/missing: No such file or directory
Test missing finished with exit code 127.

/tmp/artifacts/PASS_good.log

Test name: good
Test directory: /var/str/.
Test command: /bin/true
Test good finished with exit code 0.

/tmp/artifacts/test.log

PASS good
FAIL bad
ERROR missing (problem with test execution)

@psss, I am fine, but it would be nice to use --args in this particular case:

--testdir="/path" for TEST_DIR
--artifactsdir="/path/2" for TEST_ARTIFACTS
--testname="xxx test 1" for TEST_NAME
-- cmd for TEST_CMD

Example:

run-basic-test --testdir="/path" --artifactsdir="/path/2" --testname="xxx test 1" -- cmd --options for cmd1 --any --format -a -o -v -d

User will be able call this command directly. Test it. Without setting ENV vars each time.

  • We have a issue to not set TEST env variable. TEST_NAME is very close and could be again collision.

And maybe use:

exec 3>&1 4>&2 1> >(tee -a "$logfile_stdout" >&3) 2> >(tee -a "$logfile_stderr" >&4)

Than logs will go to STDOUR/STDERR as expected + log files.

and will will remove this code from playbook:

       TEST_NAME:
          "{{ item if item.keys is not defined else (item.keys()|list)[0] }}"
       TEST_DIR:
         "{{ tenv_workdir }}{{ item if item.keys is not defined else item[(item.keys()|list)[0]]['dir']|default((item.keys()|list)[0]) }}"
        TEST_CMD:
          "{{'./runtest.sh' if item.keys is not defined else item[(item.keys()|list)[0]]['run']|default('./runtest.sh')}}"
       TEST_ARTIFACTS:
         "{{ remote_artifacts }}"

Please. If you do not want, I will re-write it quickly, and you change format output according to discussion about (yaml).

OK, feel free to adjust as needed. I don't see much benefit in implementing the option parsing as I believe users will not be using this helper scripts directly. Also I would prefer both stdout and stderr rather in a single for easier investigation. But this is probably a matter of personal taste. Finally, I'm not sure how you would like to remove the item if item.keys... section from the playbook: It's necessary to check test items and pick dir and run attributes. On the other hand I would agree to remove the undocumented "guessing" part of this.

we have issue: https://pagure.io/standard-test-roles/issue/83
Yes, it is for Beakerlib. But let be consistent. stdout goes stdout. and stderr goes to stderr.
Ok I will update. Please wait.

Does that mean that beakerlib is sending output to both stdout and stderr? If so, I'd suggest to fix this in beakerlib rather than introducing two files for user to inspect.

Regarding basic role I would suggest to only have a single file with both stdout and stderr as this is IMO easier for debugging. As far as BeakerLib is concerned, duplicated lines are weird issue and if we cannot find a better way how to fix it, separating stdout and stderr makes it probably a bit better (but still ugly).

@astepano, this is blocking test results improvements proposed in #306 filed at the beginning of March. Could we move this forward in the near future? Thanks.

Pull-Request has been closed by astepano

Obsoleted by #335.

Metadata