standard-test-roles

Clone
Source Code
GIT

#382 Basic&beakerlib take into account external termination.
Merged 3 years ago by astepano. Opened 3 years ago by astepano.
ext_term  into  master

file modified
+42 -10 
@@ -109,24 +109,42 @@ 
 

  # Starting from this point and bellow any fail is considered as a test fail. Exit code == 0.
 

  

  clean_exit() {
 

-     rc=$?;
 

+     rc=$?
 

+     # WARNING! At this place ansible closes all FD for STDIN STDERR.
 

+     # echo "something" > ANY will not work, and will fail
 

+     # With the above do next relax:
 

+     set +efu
 

+     # Also any output to old tee-STDERR/STDOUT will terminate clean_exit()
 

      trap - SIGINT SIGTERM SIGABRT EXIT # clear the trap
 

-     echo "Run test '$STR_TEST_NAME': done. Test's exit code: $rc"
 

+     echo "Run test '$STR_TEST_NAME': done. Test's exit code: $rc" >&4
 

+     if [[ $terminated_outside -eq 1 ]]; then
 

+         echo "The test was terminated outside." >&4
 

+         echo "Mark current test as ERROR." >&4
 

+         rc=77
 

+     fi
 

      # 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
 

+     for pid in $(ps -o pid --no-headers --ppid $$ 2>/dev/null); do
 

+         if [ -n "$(ps -p $pid -o pid= 2>/dev/null)" ]; then
 

+             kill -s HUP $pid > /dev/null 2>&1
 

          fi
 

      done
 

+     # At this place STDIN/STDOUT(tee) are closed.
 

+     # Can work original STDOUT/STDERR &3 and &4.
 

+     # Depends how this command was invoked.
 

      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
 

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

+         status="ERROR"
 

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

+         # Test was terminated outside.
 

+         # For example: ansible-playbook was terminated
 

+         echo "$STR_TEST_NAME (test was terminated outside)" >&4
 

          status="ERROR"
 

      elif [[ $rc -eq 124 ]]; then
 

          # test case timed out
 

-         echo "$STR_TEST_NAME (test aborted due to timeout)" >&2
 

+         echo "$STR_TEST_NAME (test aborted due to timeout)" >&4
 

          status="ERROR"
 

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

          status="PASS"
 
@@ -158,22 +176,36 @@ 
 

          cp -f $STR_WORKDIR/$file $STR_ARTIFACTS_DIR/$STR_TEST_NAME || true
 

          set -f
 

      done
 

+     # If killed outside, return code will not be 0, no matter what.
 

      exit 0
 

  }
 

  trap clean_exit SIGINT SIGTERM SIGABRT EXIT
 

+ terminated_outside=1
 

+ rc=0
 

  

  export PATH="$PATH:$STR_WORKDIR"
 

  mkdir -p "$STR_ARTIFACTS_DIR"
 

  # add str_ prefix to test logs
 

  logfile="$STR_ARTIFACTS_DIR/$(echo "str_$STR_TEST_NAME" | sed -e 's/\//-/g').log"
 

  logfile="$(realpath "$logfile")"
 

- exec > >(tee -a "$logfile") 2>&1
 

+ # Save real STDIN/STDERR to 3 and 4
 

+ exec 3>&1 4>&2 > >(tee -a "$logfile") 2>&1
 

  cd "$STR_WORKDIR"
 

  # Purpose to spawn new bash is to ignore -efu setting for current shell
 

  # Test command: run-basic-test -w wodir -c 'false; echo 123; echo 333 >&2; touch "123 123"; exit 43' -t my_test -a logs -v
 

  if [ -f "$STR_CMD" ]; then
 

      chmod 0775 "$STR_CMD"
 

  fi
 

- timeout --foreground "$STR_TIMEOUT" bash -c "$STR_CMD"
 

+ timeout --foreground "$STR_TIMEOUT" bash -c "$STR_CMD" || rc=$?
 

+ # `terminated_outside` epxlanation:
 

+ # This is for a case when STR is driven by STR-pipeline.
 

+ # The STR-pipeline has code: `timeout 4h ansible-playbook`.
 

+ # `ansible-playbook` is terminated by `timeout` command.
 

+ # Active ansible-task (this scipt) is terminated with HUP signal.
 

+ # In this case `clean_exit()` is called with:
 

+ # Signal: HUP
 

+ # Rerurn code: 0 (the `timeout` command will be terminated itself ==> no return code from test command)
 

+ # If terminated_outside==1 mark unfinished script as failed.
 

+ terminated_outside=0
 

  # Explicit return code
 

- exit $?
 

+ exit $rc

file modified
+36 -11 
@@ -93,15 +93,28 @@ 
 

  # Starting from this point and bellow any fail is considered as a test fail. Exit code == 0.
 

  

  clean_exit() {
 

-     rc=$?;
 

+     rc=$?
 

+     # WARNING! At this place ansible closes all FD for STDIN STDERR.
 

+     # echo "something" > ANY will not work, and will fail
 

+     # With the above, relax:
 

+     set +efu
 

+     # Also any output to old tee-STDERR/STDOUT will terminate clean_exit()
 

      trap - SIGINT SIGTERM SIGABRT EXIT # clear the trap
 

-     echo "Run test $STR_BKR_TEST: done."
 

+     echo "Run test '$STR_BKR_NAME': done. Test's exit code: $rc" >&4
 

+     if [[ $terminated_outside -eq 1 ]]; then
 

+         echo "The test was terminated outside." >&4
 

+         echo "Mark current test as ERROR." >&4
 

+         rc=77
 

+     fi
 

      # 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
 

+     for pid in $(ps -o pid --no-headers --ppid $$ 2>/dev/null); do
 

+         if [ -n "$(ps -p $pid -o pid= 2>/dev/null)" ]; then
 

+             kill -s HUP $pid > /dev/null 2>&1
 

          fi
 

      done
 

+     # At this place STDIN/STDOUT(tee) are closed.
 

+     # Can work original STDOUT/STDERR &3 and &4.
 

+     # Depends how this command was invoked.
 

      # Check test result status
 

      local log_file_name="$STR_BKR_TEST_DASHED.log"
 

      local log_file_path="$STR_ARTIFACTS_DIR/$log_file_name"
 
@@ -110,7 +123,12 @@ 
 

          status="ERROR"
 

      elif [[ $rc -eq 124 ]]; then
 

          # test case timed out
 

-         echo "$STR_BKR_TEST (test aborted due to timeout)" >&2
 

+         echo "$STR_BKR_TEST (test aborted due to timeout)" >&4
 

+         status="ERROR"
 

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

+         # Test was terminated outside.
 

+         # For example: ansible-playbook was terminated
 

+         echo "$STR_TEST_NAME (test was terminated outside)" >&4
 

          status="ERROR"
 

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

          status="ERROR"
 
@@ -143,9 +161,12 @@ 
 

          fi
 

      done
 

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

+     # If killed outside, return code will not be 0, no matter what.
 

      exit 0
 

  }
 

  trap clean_exit SIGINT SIGTERM SIGABRT EXIT
 

+ terminated_outside=1
 

+ rc=0
 

  

  # For beakerlib-libraries
 

  export PATH="$PATH:$STR_WORKDIR"
 
@@ -154,6 +175,7 @@ 
 

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

  # OUTPUTFILE has influence on beakerlib-libraries output
 

  export OUTPUTFILE="$(realpath "$logfile_stdout")"
 

+ # Save real STDIN/STDERR to 3 and 4
 

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

  mkdir -p "$STR_WORKDIR"
 

  cd "$STR_WORKDIR"
 
@@ -161,14 +183,16 @@ 
 

  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"
 

+     terminated_outside=0
 

      exit 127
 

  fi
 

  

  if [ -f "$STR_BKR_TEST" ]; then
 

      debug "Running test from file: $STR_BKR_TEST"
 

      cd $(dirname "$STR_BKR_TEST")
 

-     timeout --foreground "$STR_TIMEOUT" /bin/sh -e ./$(basename "$STR_BKR_TEST")
 

-     exit
 

+     timeout --foreground "$STR_TIMEOUT" /bin/sh -e ./$(basename "$STR_BKR_TEST") || rc=$?
 

+     terminated_outside=0
 

+     exit $rc
 

  fi
 

  

  if [ -d "$STR_BKR_TEST" ]; then
 
@@ -178,12 +202,13 @@ 
 

      get-test-deps -i .
 

      if [ -f "Makefile" ] && command -p -v "make" >"/dev/null" 2>&1; then
 

          debug "Running test from Makefile"
 

-         timeout --foreground "$STR_TIMEOUT" make run
 

+         timeout --foreground "$STR_TIMEOUT" make run || rc=$?
 

      elif [ -f "runtest.sh" ]; then
 

          debug "Running test from runtest.sh"
 

-         timeout --foreground "$STR_TIMEOUT" /bin/sh -e ./runtest.sh
 

+         timeout --foreground "$STR_TIMEOUT" /bin/sh -e ./runtest.sh || rc=$?
 

      else
 

          echo "FAIL test $STR_BKR_TEST do not know how to run test"
 

      fi
 

-     exit
 

+     terminated_outside=0
 

+     exit $rc
 

  fi

file modified
-1 
@@ -4,4 +4,3 @@ 
 

  pass2
 

  pass3
 

  error3
 

- Run test 'test-basic-stdout-stderr': done. Test's exit code: 0

Why this line needs to be removed?

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

@bgoncalv
The pipeline has code:

timeout 4h ansible-playbook -v --inventory=pipeline_inventory.yaml --extra-vars ansible_python_interpreter=/usr/bin/python3 --tags classic tests.yml

I would like to ask you to test this PR for this case with command:

timeout 3min ansible-playbook tests.yml

both for basic + beakerlib roles

Make sure that the the running test is reported as ERROR.
Current STR without this PR should mark running test as PASS.

Thank you.

pretty please pagure-ci rebuild

@astepano I've tried with standard-test-roles-4.4-1.fc31.602680d.1.noarch the tests now don't have the PASS prefix, but they don't have ERROR either, they don't contain any prefix.

# ls /tmp/artifacts/
str_test-abort-playbook-basic.log  test-abort-playbook-beakerlib-err.log  test-abort-playbook-beakerlib.log

timeout 2m ansible-playbook --inventory=pipeline_inventory.yaml --extra-vars ansible_python_interpreter=/usr/bin/python3 --tags classic timeout_basic.yml

basic playbook:

# Make sure the role report error when playbook aborts
- hosts: localhost
tags:
- classic
roles:
- role: standard-test-basic
    tests:
    - test-abort-playbook-basic:
        run: sleep 240

timeout 2m ansible-playbook --inventory=pipeline_inventory.yaml --extra-vars ansible_python_interpreter=/usr/bin/python3 --tags classic timeout_beakerlib.yml

beakerlib playbook

# Make sure the role report error when playbook aborts
- hosts: localhost
tags:
- classic
roles:
- role: standard-test-beakerlib
    tests:
    - test-abort-playbook-beakerlib:
        run: sleep 240

test-abort-playbook-beakerlib/runtest.sh

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
#!/bin/bash
# Include Beaker environment
. /usr/share/beakerlib/beakerlib.sh || exit 1

PACKAGE="bash"

rlJournalStart
    rlPhaseStartTest "test abort playbook with beakerlib role"
        rlRun "sleep 240"
    rlPhaseEnd
rlJournalPrintText
rlJournalEnd

rebased onto 1391eea1334328bbc585baa8bd83577304a1b82d
3 years ago

Updated. @bgoncalv
It was hard. I do not know how it worked before.
Please test again for basic&beakerlib in different ways.
Thank you.

rebased onto d1b05453fe576be7faff579f0e6c32fd818a3736
3 years ago

rebased onto 0412f64
3 years ago

@astepano thanks a lot for this work! I've tested with standard-test-roles-4.4-1.fc31.0412f64.1.noarch and basic and beakerlib roles were aborted succesfully, the tests got ERROR prefix and results.yml was correctly created!

Just a note with official STR standard-test-roles-4.5-3.fc31.noarch beakerlib role was already reporting aborted test with ERROR prefix correctly, so basically it would be your decision if you want use this fix for bekerlib role or not.

Why this line needs to be removed?

That string comes not from test itself.
That string comes from cleaner.
That log supposed to have output from test.
I would like to keep that string, but:

with this PR that string doesn't go to that file.
in that place (in cleaner) no reliable way to to print any data to log at at that moment.
because it happens during cleanup, and tee process can be terminated at that point.

Could you please check current (without PR) test for beakerlib.
Without timeout:
ansible-playbook ....
And press CTRL-C, will it have ERROR too? If yes, then I will remove changes to beakerlib.
Thank you.

CTRL-C at during task-run Run beakerlib tests

CTRL-C at during task-run Run beakerlib tests

Cool, the CTRL-C only works with the patch.

Commit 5589c7b fixes this pull-request

Pull-Request has been merged by astepano
3 years ago

Pull-Request has been merged by astepano
3 years ago
Changes Summary 3
+42 -10
file changed
roles/standard-test-basic/files/run-basic-test
+36 -11
file changed
roles/standard-test-beakerlib/files/run-beakerlib-test
+0 -1
file changed
tests/test-basic-stdout-stderr/expected_output.txt
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.