standard-test-roles

Clone
Source Code
GIT

#306 Add support for results.yml [fix #296]
Merged 5 years ago by astepano. Opened 5 years ago by psss.
psss/standard-test-roles results  into  master

file modified
+8 -3 
@@ -106,16 +106,21 @@ 
 

              kill -s HUP $pid
 

          fi
 

      done
 

+     local status="FAIL"
 

      # Return non-zero when test command not found
 

      if [[ $rc -eq 127 ]]; then
 

          echo "$STR_TEST_NAME (problem with test execution)" >&2
 

-     fi
 

-     local status="FAIL"
 

-     if [[ $rc -eq 0 ]]; then
 

+         status="ERROR"
 

+     elif [[ $rc -eq 0 ]]; then
 

          status="PASS"
 

      fi
 

      local run_journal="$STR_ARTIFACTS_DIR/test.log"
 

      echo "${status} $STR_TEST_NAME" >> "$run_journal"
 

+     # Handle results.yml file
 

+     local results="$STR_ARTIFACTS_DIR/results.yml"
 

+     local result=$(echo $status | tr '[:upper:]' '[:lower:]')
 

+     test -f "$results" || echo 'results:' > "$results"
 

+     echo "- {result: $result, test: $STR_TEST_NAME}" >> "$results"
 

      if [ -e "$logfile_stdout" ]; then
 

          local new_logfile_stdout="$(dirname "$logfile_stdout")/${status}_str_$(basename "$logfile_stdout")"
 

          mv -f "$logfile_stdout" "$new_logfile_stdout"

file modified
+4 -4 
@@ -14,15 +14,15 @@ 
 

    # Can't go in block. See
 

    # https://github.com/ansible/ansible/issues/20736
 

    - name: Check the results
 

-     shell: grep "^FAIL" {{ remote_artifacts }}/test.log
 

-     register: test_fails
 

+     shell: grep "^ERROR" {{ remote_artifacts }}/test.log
 

+     register: test_error
 

      # Never fail at this step. Just store result of tests.
 

      failed_when: False
 

  

    - name: Set role result
 

      set_fact:
 

-       role_result_failed: "{{ (test_fails.stdout|d|length > 0) or (test_fails.stderr|d|length > 0) }}"
 

-       role_result_msg: "{{ test_fails.stdout|d('tests failed.') }}"
 

+       role_result_error: "{{ (test_error.stdout|d|length > 0) or (test_error.stderr|d|length > 0) }}"
 

+       role_result_msg: "{{ test_error.stdout|d('test execution error.') }}"
 

  

    - include_role:
 

        name: str-common-final

file modified
+32 -5 
@@ -80,6 +80,7 @@ 
 

  debug "Test: $STR_BKR_TEST"
 

  debug "Work dir: $STR_WORKDIR"
 

  debug "Artifacts dir: $STR_ARTIFACTS_DIR"
 

+ STR_BKR_TEST_DASHED=$(echo "$STR_BKR_TEST" | sed -e 's/\//-/g')
 

  

  # Up to this point any fail is considered as ci-sytem fail. Exit code != 0.
 

  # Starting from this point and bellow any fail is considered as a test fail. Exit code == 0.
 
@@ -88,13 +89,39 @@ 
 

      rc=$?;
 

      trap - SIGINT SIGTERM SIGABRT EXIT # clear the trap
 

      echo "Run test $STR_BKR_TEST: done."
 

-     # Exit code == 0, no matter of the test result.
 

      # Close tee pipes
 

      for pid in $(ps -o pid --no-headers --ppid $$); do
 

          if [ -n "$(ps -p $pid -o pid=)" ]; then
 

              kill -s HUP $pid
 

          fi
 

      done
 

+     # Check test result status
 

+     local log_file_name="$STR_BKR_TEST_DASHED.log"
 

+     local log_file_path="$STR_ARTIFACTS_DIR/$log_file_name"
 

+     local status
 

+     if [[ $rc -eq 127 ]]; then
 

+         status="ERROR"
 

+     elif grep -q "RESULT: WARN" "$log_file_path"; then
 

+         status="ERROR"
 

+     elif grep -q "RESULT: FAIL" "$log_file_path"; then
 

+         status="FAIL"
 

+     elif grep -q "RESULT: PASS" "$log_file_path"; then
 

+         status="PASS"
 

+     elif grep -q "FAIL" "$log_file_path"; then
 

+         status="FAIL"
 

+     elif grep -q "PASS" "$log_file_path"; then
 

+         status="PASS"
 

+     else
 

+         status="ERROR"
 

+     fi
 

+     echo "$status $STR_BKR_TEST" >> "$STR_ARTIFACTS_DIR/test.log"
 

+     mv "$log_file_path" "$STR_ARTIFACTS_DIR/${status}_${log_file_name}"
 

+     # Handle results.yml file
 

+     local results="$STR_ARTIFACTS_DIR/results.yml"
 

+     local result=$(echo $status | tr '[:upper:]' '[:lower:]')
 

+     test -f "$results" || echo 'results:' > "$results"
 

+     echo "- {result: $result, test: $STR_BKR_TEST}" >> "$results"
 

+     # Exit code == 0, no matter of the test result.
 

      exit 0
 

  }
 

  trap clean_exit SIGINT SIGTERM SIGABRT EXIT
 
@@ -103,9 +130,9 @@ 
 

  export PATH="$PATH:$STR_WORKDIR"
 

  mkdir -p "$STR_ARTIFACTS_DIR"
 

  # OUTPUTFILE has influence on beakerlib-libraries output
 

- export OUTPUTFILE="$(realpath "$STR_ARTIFACTS_DIR")/$(echo "$STR_BKR_TEST" | sed -e 's/\//-/g')-out.log"
 

- logfile_stdout="$STR_ARTIFACTS_DIR/$(echo "$STR_BKR_TEST" | sed -e 's/\//-/g').log"
 

- logfile_stderr="$STR_ARTIFACTS_DIR/$(echo "$STR_BKR_TEST" | sed -e 's/\//-/g')-err.log"
 

+ export OUTPUTFILE="$(realpath "$STR_ARTIFACTS_DIR")/$STR_BKR_TEST_DASHED-out.log"
 

+ logfile_stdout="$STR_ARTIFACTS_DIR/$STR_BKR_TEST_DASHED.log"
 

+ logfile_stderr="$STR_ARTIFACTS_DIR/$STR_BKR_TEST_DASHED-err.log"
 

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

  mkdir -p "$STR_WORKDIR"
 

  cd "$STR_WORKDIR"
 
@@ -113,7 +140,7 @@ 
 

  if ! [ -d "$STR_BKR_TEST" ] && ! [ -f "$STR_BKR_TEST" ]; then
 

      # Next string goes to .log file
 

      echo "FAIL test $STR_BKR_TEST does not appear to be a file or directory"
 

-     exit 1
 

+     exit 127
 

  fi
 

  

  if [ -f "$STR_BKR_TEST" ]; then

file modified
+4 -26 
@@ -60,40 +60,18 @@ 
 

        BEAKERLIB_LIBRARY_PATH: "{{ beakerlib_libraries_path }}"
 

  

    always:
 

-   - name: Make the master tests summary log artifact
 

-     shell: |
 

-       log_file_name=$(echo {{ item }} | sed -e 's/\//-/g').log
 

-       logfile={{ remote_artifacts }}/${log_file_name}
 

-       status="FAIL"
 

-       if grep -q '\[ *FAIL *\]' "$logfile"; then
 

-         status="FAIL"
 

-       elif grep -q '\[ *PASS *\]' "$logfile"; then
 

-         status="PASS"
 

-       elif grep -q FAIL "$logfile"; then
 

-         status="FAIL"
 

-       elif grep -q PASS "$logfile"; then
 

-         status="PASS"
 

-       else
 

-         status="FAIL"
 

-       fi
 

-       echo "${status} {{ item }}" >> {{ remote_artifacts }}/test.log
 

-       mv ${logfile} {{ remote_artifacts }}/${status}_${log_file_name}
 

-     with_items:
 

-     - "{{ tests }}"
 

-     - "{{ filter_tests }}"
 

- 

    # Can't go in block. See
 

    # https://github.com/ansible/ansible/issues/20736
 

    - name: Check the results
 

-     shell: grep "^FAIL" {{ remote_artifacts }}/test.log
 

-     register: test_fails
 

+     shell: grep "^ERROR" {{ remote_artifacts }}/test.log
 

+     register: test_error
 

      # Never fail at this step. Just store result of tests.
 

      failed_when: False
 

  

    - name: Set role result
 

      set_fact:
 

-       role_result_failed: "{{ (test_fails.stdout|d|length > 0) or (test_fails.stderr|d|length > 0) }}"
 

-       role_result_msg: "{{ test_fails.stdout|d('Beaker tests failed.') }}"
 

+       role_result_error: "{{ (test_error.stdout|d|length > 0) or (test_error.stderr|d|length > 0) }}"
 

+       role_result_msg: "{{ test_error.stdout|d('test execution error.') }}"
 

  

    - include_role:
 

        name: str-common-final

file modified
+2 -2 
@@ -11,8 +11,8 @@ 
 

  - name: Report role result
 

    vars:
 

      msg: |
 

-        Tests failed: {{ role_result_failed|d('Undefined') }}
 

+        Tests error: {{ role_result_error|d('Undefined') }}
 

         Tests msg: {{ role_result_msg|d('None') }}
 

    debug:
 

      msg: "{{ msg.split('\n') }}"
 

-   failed_when: "role_result_failed|bool"
 

+   failed_when: "role_result_error|bool"

Added support for results.yaml to basic and beakerlib role and removed non-zero exit status reporting. @astepano, could you please review if something like this would be OK? I would then continue with other roles as well.

Also, when disabling the non-zero exit status I've noticed that role_result_failed is only supported by these two roles. Is that expected? Thanks.

Resolves #296.

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?

1 new commit added

  • Use quotes for remote_artifacts and results
5 years ago

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?
Edited 5 years ago by astepano

@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 onto f110e90ab03b2897a265e4643629d867d279fe92
5 years ago

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) ?
Edited 5 years ago by astepano

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.

rebased onto 466d50a
5 years ago

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

Seems reasonable.
Merging, @psss thank you!

Commit 9d87c81 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 5
+8 -3
file changed
roles/standard-test-basic/files/run-basic-test
+4 -4
file changed
roles/standard-test-basic/tasks/main.yml
+32 -5
file changed
roles/standard-test-beakerlib/files/run-beakerlib-test
+4 -26
file changed
roles/standard-test-beakerlib/tasks/main.yml
+2 -2
file changed
roles/str-common-final/tasks/main.yml
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.