Detect test failures in dependent roles

asmartin commented 9 years ago

I've implemented additional functionality on top of rolespec that allows you to test a role which has dependent roles defined in its roles/<role>/meta/main.yml:


---
dependencies:
  - { role: other1 }
  - { role: other2 }

What I did was separate out tests/<role>/test into two files:

tests/thisrole/test:

#!/bin/bash
# source tests for all dependent roles here
source tests/other1/test_list
source tests/other2/test_list

# tests for this role
source tests/thisrole/test_list

tests/thisrole/test_list:

assert_path /usr/bin/vim

assert_in_file /etc/ntp.conf "ntp.org"

Thus, when you call rolespec on the thisrole role, it will call the tests for the dependent roles in addition to the tests for thisrole specifically. This works well, however the summary functionality that prints "Test completed successfully!!" at the end cannot currently detect failures in dependent role tests, so it will report that all tests completed successfully even if some dependent tests are listed as FAIL. Would it be possible to add this functionality to detect failures in dependent roles?

Please let me know if you would be interested in merging this functionality and I can elaborate on the parts I have implemented already.

nickjj commented 9 years ago

Hmm, that's an interesting approach but I'm not sure I'm sold on it yet. My concern is let's say I'm testing a rails role. This rails role might use postgresql, redis, ruby, nginx and so on.

When I want to test the rails role, I only care that the rails portion of it is being tested. If I want to test nginx then I would run tests against the nginx role.

Failed tests should also instantly stop the script by exiting with 1. I'm wondering how it's even getting to the end to throw out the success message while tests have failed.

asmartin commented 9 years ago

I can see both cases, depending on the specific role being tested. In some cases it may be desirable to test how different roles interact together, in which case it would be useful to be able to source an existing lists of tests. (by adding them to tests/<role>/test). In other cases, as with the rails example, you may just want to test the root role (in which case you don't add any dependent roles to tests/<role>test). For my use case it is nice to have this flexibility.

Regarding the problem where dependent roles fail, I do not have set -e added to tests/<role>/test - is that needed here in order to cause it to abort after any failures, or is that handled elsewhere by rolespec?

asmartin commented 9 years ago

It looks like the lack of failure when a test fails is due to how Bash's source command works:

       source filename [arguments]
              Read  and  execute  commands  from filename in the current shell environment and return the exit status of the last command executed from filename.  If filename
              does not contain a slash, filenames in PATH are used to find the directory containing filename.  The file searched for in PATH need  not  be  executable.   When
              bash  is not in posix mode, the current directory is searched if no file is found in PATH.  If the sourcepath option to the shopt builtin command is turned off,
              the PATH is not searched.  If any arguments are supplied, they become the positional parameters when filename is executed.  Otherwise the positional  parameters
              are unchanged.  The return status is the status of the last command exited within the script (0 if no commands are executed), and false if filename is not found
              or cannot be read.

Thus, if a function in the sourced file exits with 1, it doesn't cause the parent script to exit, at most it would just set the return code from the source call to 1. The way to resolve this is to use set -e at the top of tests/<role>/test, which then causes immediate exit on any non-zero exit call.

Would you be interested in a patch for this?

Also, would you consider a patch for creating the separate tests/<role>/test and tests/<role>/test_list files for including tests for dependent roles in cases that it makes sense (an optional feature that you can add to certain roles as desired)? I've actually extended this further, so that you have the following

tests\
     header
     <role>
            \test
            \test_list

Definitions:

header - a global file that is sourced in all tests/<role>/test files that contains common code and some utility functions that can be used in each test, e.g test_setup which prints out a description of the test before it is run:

# exit immediately if there are any errors
set -e

# This gives you access to the custom DSL
. "${ROLESPEC_LIB}/main"

# set the TERM variable so tput and other utilities will work correctly
export TERM=linux

## prepares for a test to be run
# $1 the message to print before running the test
function test_setup() {
        # print a line the width of the terminal to separate this test
        printf '%*s\n' "80" '' | tr ' ' -
        printf '***** %s *****\n' "$1"
}

# Everything past this point is the custom DSL which is optional

# Install a specific version of Ansible, you can
# omit the version to install devel (latest unstable)
install_ansible "v1.7.2"

# required for assert_url
install curl

# An assertion which does the following:
#   - Syntax check on the playbook
#   - Run the playbook
assert_playbook_runs $OPTS

tests/<role>/test - the main test file that is called by rolespec; it sources header, it can source any dependent role tests (but this is optional), and then it sources the test_list for this role:

#!/bin/bash
# put any arguments to pass to ansible-playbooks here
OPTS=""

# load global test header
source tests/header

# enable this to verify that re-runs the playbook checking for no changes
#assert_playbook_idempotent

# source tests for all dependent roles here, e.g
source tests/apache2/test_list

# tests for this role
source tests/apache2-ssl/test_list

tests/<role>/test_list - this file contains all of the tests for this role:

# put tests for this role here
test_setup "verify that /etc/php5/apache2/php.ini has been modified"
assert_in_file /etc/php5/apache2/php.ini "Ansible managed"

test_setup "verify that php5 apache module is loaded"
assert_path /etc/apache2/mods-enabled/php5.load

nickjj commented 9 years ago

How would you patch #1? If it happens inside of the test script then that's already user land. The main rolespec script already uses set -e, this is how it calls the test script -- https://github.com/nickjj/rolespec/blob/master/bin/rolespec#L95.

As for #2 have you checked this yet? https://github.com/debops/test-suite/blob/master/defaults.conf, basically we use that as a config with shared code for all of our tests. We just source it in each test.

I'm concerned about your test_list idea for travis-based tests. When it's local you have all of the roles, but with travis it only clones the role that's being ran. In your example let's say the travis config is in the php5 role, it would have no idea about the apache role. How do you plan to solve this without any additional configuration for the user?

asmartin commented 9 years ago

I would patch #1 by just adding set -e at the top of tests/<role>/test - I've tested this and it now works as expected (failures in dependent roles stop the test). I think the problem with how it is implemented currently in https://github.com/nickjj/rolespec/blob/master/bin/rolespec#L95 is that bash doesn't pass variables set with set down to subshells. Try this as a test:

one.sh:

#!/bin/bash
set -x
echo "this is one"
./two.sh
echo "this is one again"

two.sh:

#!/bin/bash
echo "this is two"

Then, call bash one.sh - if set -x were passed to two.sh, then the echo "this is two" line should be echoed before it is executed. However, that does not occur:

+ echo 'this is one'
this is one
+ ./two.sh
this is two
+ echo 'this is one again'
this is one again

So similarly, even though set -e is set in bin/rolespec, when ${ROLESPEC_SCRIPT} is called, it does not inherit set -e.

For #2, you're right, https://github.com/debops/test-suite/blob/master/defaults.conf achieves the same effect as the header file I created.

I'm unfamiliar with Travis - is it possible to pass a list of directories to copy into the Travis environment? In the Dockerfile I use, I just add the entire roles and tests directories:

ADD roles ${WORKDIR}/roles
ADD tests ${WORKDIR}/tests

If it were possible, then you could just configure it to copy in this additional data for your test. If not, you could have a function that detects if the test is running in Travis and if so excludes dependent role testing:

if ! is_travis; then
    source tests/mydependentrole/test_list
fi

source tests/myrole/test_list

nickjj commented 9 years ago

Ah right, I forgot I wrote the init script to create a test script skeleton. That's what you want to patch right for #1? I would be ok with that PR.

As for #2...

You can't copy directories but you could get the files on there by just cloning repos or running galaxy install on the roles you need.

I already have a routine setup to pick out dependencies from your meta file automatically and then galaxy install them. You probably haven't seen this run yet if you're only doing local tests.

That code is here: https://github.com/nickjj/rolespec/blob/master/lib/dsl/ansible#L37-L48

asmartin commented 9 years ago

Yes, for #1.

As for #2, ah, yes all of my roles are local and not published to Ansible Galaxy so I had not encountered that bit of code. If you'd prefer to just keep it focused on Ansible Galaxy and Travis as the two cases that rolespec supports that's fine with me, I just thought I'd try to upstream the enhancements I've made to make it work in my (entirely local) environment

nickjj / rolespec

Detect test failures in dependent roles #4