#367 support to save files on basic role
Merged 4 years ago by astepano. Opened 4 years ago by bgoncalv.
bgoncalv/standard-test-roles fix-role  into  master

@@ -23,6 +23,9 @@ 

   * **dir**: test directory. default: is the test name

   * **run**: command to run the test. default: ./runtest.sh

   * **timeout**: abort test case after this time. More details on [timeout][1]. default: 0

+  * **save-files**: List of extra files to save to artifacts.

+    Path to the file is relative to test directory. The files are saved to

+    artifacts under test name directory. default: None

  

  Example usage:

  

@@ -25,6 +25,7 @@ 

  -t, --testname          name of the test

      --timeout           test timeout

  -c, --cmd               shell command to run the test

+     --save-files        list of files to be saved to artifacts, files are separated by comma. Path is relative to workdir

  EOF

  }

  
@@ -39,9 +40,10 @@ 

  STR_TEST_NAME="${STR_TEST_NAME:-}"

  STR_ARTIFACTS_DIR="${STR_ARTIFACTS_DIR:-/tmp}"

  STR_TIMEOUT="${STR_TIMEOUT:-0}"

+ STR_SAVE_FILES="${STR_SAVE_FILES:-}"

  

  # http://wiki.bash-hackers.org/howto/getopts_tutorial

- opt=$(getopt -n "$0" --options "hvt:w:a:c:" --longoptions "help,verbose,cmd:,testname:,workdir:,artifactsdir:,timeout:" -- "$@")

+ opt=$(getopt -n "$0" --options "hvt:w:a:c:" --longoptions "help,verbose,cmd:,testname:,workdir:,artifactsdir:,timeout:,save-files:" -- "$@")

  eval set -- "$opt"

  while [[ $# -gt 0 ]]; do

      case "$1" in
@@ -65,6 +67,10 @@ 

              STR_TIMEOUT="$2"

              shift 2

              ;;

+         --save-files)

+             STR_SAVE_FILES=$(echo "$2" | tr "," " ")

+             shift 2

+             ;;

          -v|--verbose)

              DEBUG="-v"

              shift
@@ -143,6 +149,15 @@ 

              echo "  - $(basename $prefixed_log)" >> "$results"

          fi

      done

+     for file in $STR_SAVE_FILES; do

+         if [[ ! -d $STR_ARTIFACTS_DIR/$STR_TEST_NAME ]]; then

+             mkdir -p $STR_ARTIFACTS_DIR/$STR_TEST_NAME

+         fi

+         set +f # allow expand wildcards on file name, like output*.log

+         # if for some reason file doesn't exist, just ignore it

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

+         set -f

+     done

      exit 0

  }

  trap clean_exit SIGINT SIGTERM SIGABRT EXIT

@@ -19,7 +19,8 @@ 

                  --artifactsdir "{{ remote_artifacts }}" \

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

                  --timeout "{{ '0' if item.keys is not defined else item[(item.keys()|list)[0]]['timeout']|default('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('\$', '\\$') }}"

+                 --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('\$', '\\$') }}" \

+                 --save-files "{{ '' if item.keys is not defined else item[(item.keys()|list)[0]]['save-files']|default('') | join(',') | regex_replace('\\', '\\\\') | regex_replace('\"', '\"') | regex_replace('\$', '\\$') }}"

if you replace join(',') with join(' ') this also should work, but no need then : do : tr

yes, it would work. I just don't like to pass to bash a parameter with spaces. But if you prefer I can do this change.

      with_items:

      - "{{ tests }}"

  

file modified
+33
@@ -91,3 +91,36 @@ 

    - import_tasks: shared-tasks/verify_error_test.yml

    - import_tasks: shared-tasks/artifacts_test_env.yml

    - import_tasks: shared-tasks/artifacts_test_runner.yml

+ 

+ # Make sure the role passes on timeout

+ - hosts: localhost

+   tags:

+   - atomic

+   - classic

+   - container

+   roles:

+   - role: standard-test-basic

+     tests:

+     - test-basic-save-files:

+         run: echo "file1" > file1.log; echo "file2" > file2.log

+         save-files:

+           - file*.log

+     - basic-save-files-dir:

+         dir: test-basic-save-files-dir

+         save-files:

+           - logs/newfile*.log

+   tasks:

+   - name: "Check if {{artifacts}}/test-basic-save-files/file1.log was created properly on test runner"

+     shell: "ls {{artifacts}}/test-basic-save-files/file1.log"

+     delegate_to: localhost

+   - name: "Check if {{artifacts}}/test-basic-save-files/file2.log was created properly on test runner"

+     shell: "ls {{artifacts}}/test-basic-save-files/file2.log"

+     delegate_to: localhost

+   - name: "Check if {{artifacts}}/basic-save-files-dir/newfile1.log was created properly on test runner"

+     shell: "ls {{artifacts}}/basic-save-files-dir/newfile1.log"

+     delegate_to: localhost

+   - name: "Check if {{artifacts}}/basic-save-files-dir/newfile2.log was created properly on test runner"

+     shell: "ls {{artifacts}}/basic-save-files-dir/newfile2.log"

+     delegate_to: localhost

+   - import_tasks: shared-tasks/artifacts_test_env.yml

+   - import_tasks: shared-tasks/artifacts_test_runner.yml

@@ -0,0 +1,7 @@ 

+ #!/usr/bin/bash

+ 

+ mkdir logs

+ 

+ echo "newfile1" > logs/newfile1.log

+ echo "newfile2" > logs/newfile2.log

+ 

Do you think it is needed? Note STR_ARTIFACTS_DIR is already created at the begin of the script.

if you replace join(',') with join(' ') this also should work, but no need then : do : tr

yes, it would work. I just don't like to pass to bash a parameter with spaces. But if you prefer I can do this change.

@bgoncalv looks good, please add --save-files entry to msg_usage() { something like:

--save-files        file blobs, listed through comma. Related to directory $WORK_DIR OR test-dir (?? please write what is necessary)

And, could you please add a test, where we run this role for test, that has dir parameter ? Just to check it works correctly when we need to cd to test-dir

rebased onto 552d85a43fd5027aba58db61d45f4ff894f678c6

4 years ago

rebased onto 3cdeea1

4 years ago

@bgoncalv looks good, please add --save-files entry to msg_usage() { something like:
--save-files file blobs, listed through comma. Related to directory $WORK_DIR OR test-dir (?? please write what is necessary)

And, could you please add a test, where we run this role for test, that has dir parameter ? Just to check it works correctly when we need to cd to test-dir

nice catch, I've updated it and added a new test.

@bgoncalv thank you.
Merging.
How about add the same approach for beakerlib role ?

Commit 6cdf434 fixes this pull-request

Pull-Request has been merged by astepano

4 years ago

Pull-Request has been merged by astepano

4 years ago

@bgoncalv thank you.
Merging.
How about add the same approach for beakerlib role ?

beakerlib has a function to archive the logs, I'm not sure what needs to be done for us to make it work on our environment. Maybe @jheger can help with info for it. I think it is this issue https://pagure.io/standard-test-roles/issue/35

As I stated in the previous issue there is a function in beakerlib called rlFileSubmit, usually used when you want to store a file from a testing environment into whatever system that runs your tests.
There is a default implementation in beakerlib that just copies specified file to /tmp on the tested machine, which is not very useful but behaviour of this function can be changed.
You can write script doing anything you want and then put a path to in BEAKERLIB_COMMAND_SUBMIT_LOG environmental variable. This new script could copy the files to the artifacts directory.