standard-test-roles

Clone
Source Code
GIT

#337 Fix a case for basic role, where run is complex bash-script
Merged 5 years ago by astepano. Opened 5 years ago by astepano.
basic_role_as_script2  into  master

file modified
+17 -6 
@@ -1,13 +1,24 @@ 
 

  ---
 

  

+ # If you make a change, then test with next playbook:
 

+ # -hosts: localhost
 

+ #   roles:
 

+ #   - role: standard-test-basic
 

+ #     tags:
 

+ #     - classic
 

+ #     tests:
 

+ #     - test one:
 

+ #         dir: tdir for test 1
 

+ #         run: echo '$'; echo '\$'; echo a1; echo "a$2"; a3="a3\na3"; echo -e ${a3}; echo \$a4; echo '$a5'; echo ">\"a6\"<"; echo "a7'\""; echo "(a8)" ; (echo "a9"); echo $(echo "a10"); echo 'a11$a11"'; echo '\\a12\\'; echo '"a13';false
 

+ 

  - block:
 

    - name: Execute tests
 

-     script:
 

-         run-basic-test -v
 

-                 --workdir {{ tenv_workdir | regex_escape() }}/{{ item if item.keys is not defined else item[(item.keys()|list)[0]]['dir']|default((item.keys()|list)[0]) | regex_escape() }}
 

-                 --artifactsdir {{ remote_artifacts | regex_escape() }}
 

-                 --test {{ item if item.keys is not defined else (item.keys()|list)[0] | regex_escape() }}
 

-                 --cmd {{ './runtest.sh' if item.keys is not defined else item[(item.keys()|list)[0]]['run']|default('./runtest.sh') | regex_escape() }}
 

+     script: |
 

+         run-basic-test -v \
 

+                 --workdir "{{ tenv_workdir }}/{{ item if item.keys is not defined else item[(item.keys()|list)[0]]['dir']|default((item.keys()|list)[0]) }}" \
 

+                 --artifactsdir "{{ remote_artifacts }}" \
 

+                 --test "{{ item if item.keys is not defined else (item.keys()|list)[0] }}" \
 

+                 --cmd "{{ './runtest.sh' if item.keys is not defined else item[(item.keys()|list)[0]]['run']|default('./runtest.sh') | regex_replace('\\', '\\\\') | regex_replace('\"', '\"') | regex_replace('\$', '\\$') }}"
 

      with_items:
 

      - "{{ tests }}"

file modified
+17 
@@ -20,6 +20,23 @@ 
 

    - import_tasks: shared-tasks/artifacts_test_env.yml
 

    - import_tasks: shared-tasks/artifacts_test_runner.yml
 

  

+ # Tests for basic role more advanced check
 

+ - hosts: localhost
 

+   tags:
 

+   - atomic
 

+   - classic
 

+   - container
 

+   roles:
 

+   - role: standard-test-basic
 

+     tests:
 

+     - basic with advanced cmdline:
 

+         dir: ./
 

+         run: |
 

+           echo -n "Cmdline test: "; a="some text"; [ "x$a" = "xsome text" ] && (echo "Passed"; exit 0) || (echo "Failed"; exit 1)
 

+   tasks:
 

+   - import_tasks: shared-tasks/artifacts_test_env.yml
 

+   - import_tasks: shared-tasks/artifacts_test_runner.yml
 

+ 

  # Make sure the role behaves correctly if test fails
 

  - hosts: localhost
 

    tags:

Signed-off-by: Andrei Stepanov astepano@redhat.com

@psss this is fix for the issue you commented at :

https://pagure.io/standard-test-roles/pull-request/335#comment-84571

@bgoncalv @psss it would be nice if you can test it.
This took a while for me to rewrite, because of double escaping.

Waiting for ACK.

pretty please pagure-ci rebuild

Is not possible to add this test playbook to STR tests?

Is not possible to add this test playbook to STR tests?
Good idea.

The fix doesn't work when playbook uses also source role.

I think it is because the work dir changed from /var/str/source to /root/\\/var\\/str\\/source

example: of tests that fail: https://src.fedoraproject.org/rpms/cockpit/blob/master/f/tests/tests.yml

Not related to this PR, but I really dislike seeing the main log empty and then having to open the error log to see what went wrong.

# cat artifacts/FAIL_str_smoke.log 
Run test 'smoke': done. Test's exit code: 127

# cat artifacts/smoke-err.log 
bash: ./smoke: No such file or directory
smoke (problem with test execution)

@bgoncalv let's back to the current PR.
Is this PR OK?
Will you open a new bug for "source role"?

I think it is because the work dir changed from /var/str/source to /root/\\/var\\/str\\/source

I didn't change this.

Not related to this PR, but I really dislike seeing the main log empty and then having to open the error log to see what went wrong. -- this is personal preference. I gave strong explanation why we do this. Please stop comment this, this comment is unrelated to this PR. :(

@bgoncalv you mentioned this doesn't work for https://src.osci.redhat.com/rpms/pyparted/blob/rhel-8.1.0/f/tests/tests.yml

With current STR (without modifications to basic) I got:

TASK [standard-test-source : Pull down the source tarballs] ***************************************************************************************************************************************************
fatal: [localhost -> 127.0.0.1]: FAILED! => {"changed": false, "msg": "Unable to retrieve source file: pyparted-3.11.0.tar.gz", "original_sources": "/root/pyparted/tests/../sources", "sources": []}
    to retry, use: --limit @/root/pyparted/tests/tests.retry

PLAY RECAP ****************************************************************************************************************************************************************************************************
localhost                  : ok=5    changed=2    unreachable=0    failed=1

Note the output of pwd is different when the patch is applied.

reproducer:

---
- hosts: localhost
  roles:
    - role: standard-test-basic
      tags:
        - classic
      tests:
        - test:
            dir: ./
            run: pwd

without the patch

# cat artifacts/PASS_str_test.log 
/var/str
Run test 'test': done. Test's exit code: 0
PASS test

with the patch

# cat artifacts/PASS_str_test.log 
/root/\/var\/str\
Run test 'test': done. Test's exit code: 0
PASS test
Edited 5 years ago by bgoncalv

Also note the playbook above creates a test case named test. This causes the main test.log to not be created. This is not a bug caused by the patch, but a regression caused by recent basic role update.

I did a few tests and it seems to be working as expected. Although I find the way of implementation with manual special characters escaping a bit fragile. Solution using environment variables proposed in #327 seems to me as a more reliable way. But let's go forward with this patch now I don't think we should invest more time into this.

 
# rpm -qa | grep standard-test-roles
standard-test-roles-inventory-qemu-3.2-1.fc29.57c878f.1.noarch
standard-test-roles-3.2-1.fc29.57c878f.1.noarch

@psss Does this work for you?

cd ~/
curl -k -O https://jenkins-continuous-infra.apps.ci.centos.org/view/Fedora%20All%20Packages%20Pipeline/job/fedora-rawhide-image-test/lastSuccessfulBuild/artifact/Fedora-Rawhide.qcow2
git clone https://src.fedoraproject.org/rpms/cockpit.git
cd cockpit/tests
rm -rf artifacts; ANSIBLE_INVENTORY=$(test -e inventory && echo inventory || echo /usr/share/ansible/inventory) TEST_SUBJECTS=~/Fedora-Rawhide.qcow2 ansible-playbook --tags=classic tests.yml

No, it does not work. It seems that the source is not prepared a thus the test fails. But this does not seem to be related to this change. Perhaps some other recent changes in the standard-test-source role?

rebased onto 2a3534de847b7408e71681d11d5215b04d19b72e
5 years ago

@bgoncalv fixed work dir
Please test.

rebased onto 12225f811a82852e6aff861b64babe65b67591c9
5 years ago

thanks, it works for me.

rebased onto 1acebff
5 years ago

@psss @bgoncalv thanks for helping fixing, and spotting issues.

Commit 7d22d56 fixes this pull-request

Pull-Request has been merged by astepano
5 years ago

Pull-Request has been merged by astepano
5 years ago
Changes Summary 2
+17 -6
file changed
roles/standard-test-basic/tasks/main.yml
+17 -0
file changed
tests/basic.yml
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.