goss-org / goss

Quick and Easy server testing/validation
https://goss.rocks
Apache License 2.0
5.6k stars 473 forks source link

Test precedence rules and sequencing of tests (feature request) #485

Closed sshipway closed 4 years ago

sshipway commented 5 years ago

Currently, all tests are run in parallel, which is faster and efficient. However, some tests may not be able to run in parallel with others, or may need to test output created by previous tests.

Can we add optional scheduling attributes 'before' and 'after' to indicate when a test should not be started until the (successful) completion of another test?

This might be a bit of a can-o-worms as it then requires checks for circular dependencies, and a method to start a new thread once its requirements are completed.

aelsabbahy commented 4 years ago

Can you provide a real world example of this use case?

sshipway commented 4 years ago

On one system, I wanted to use Goss to test the application behaviour under tests which temporarily modify the database. There are several tests to run, but unfortunately they cannot run simultaneously as they modify the same area of data, and so they have to run sequentially.

In the end, I was able to make some major changes to the unit tests so that they could run in parallel. However it would have been easier had I been able to mark a dependency sequence to allow the tests to run consecutively.

sshipway commented 4 years ago

Another example would be a test that requires some separate test data to have been set up in advance. One test could test for the existence of the test data, and another tests for the functionality. For example, test that a certain DNS name is resolveable; if so, we can continue to test the function that depends on it. This makes it easier to focus on the root cause of a failure.

It would be useful if a whole test could be Skipped if the dependent test failed, in much the same way that a command stdout pattern test is skipped when the exit-status test fails.

aelsabbahy commented 4 years ago

First example:

  1. I don't think goss should be in the business of changing system state, rather only testing it.
  2. That said, if your tests cannot be run in parallel, this can be handled by using goss v --max-concurrent 1 For example:
    command:
    01:
    exec: sleep 1
    exit-status: 0
    02:
    exec: sleep 1
    exit-status: 0
    03:
    exec: sleep 1
    exit-status: 0
    
    $ goss v
    ...

Total Duration: 1.008s Count: 3, Failed: 0, Skipped: 0

$ goss v --max-concurrent 1 ...

Total Duration: 3.010s Count: 3, Failed: 0, Skipped: 0


Test order is not guaranteed and will be random, this can be observed by using `--format documentation`.

## Second example:
I do see some benefit of this to aid with root cause. However, it adds a lot of complexity to the task scheduler, and adds little to the user experience. Here's my high level thoughts on what would be needed and why I feel overall the cost/benefit of the effort might not be worthwhile:
1. Add dependency management.
2. Deduplication/locking of executors. Example: TaskA and TaskB depend on TaskC, only run/report C once.
3. Detection of circular dependencies.

or as you put it, it's a bit of "can-o-worms" =)

One possible alleviation of the clarity/root-cause problem would be leveraging the `meta` field, I know this isn't exactly what you're looking for, but it might help if you're not already leveraging them:
```yaml
dns:
  localhost2:
    resolveable: true
    timeout: 500
command:
  ping_google:
    # "title" and "meta" are arbitrary values that will be printed out in the test output.
    # "title" is a string value, "meta" is an arbitrary set of key/value pairs.
    # Here we are using meta to provide the user with checklist of possible remedies
    # when this test fails
    title: Check that we can ping google successfully
    meta:
      remedy.1: Check if DNS test failed first
      remedy.2: Check firewall rules
    exec: ping -c 1 -W 1 localhost2
    exit-status: 0

And the output:

$ goss v
FF

Failures/Skipped:

DNS: localhost2: resolvable:
Expected
    <bool>: false
to equal
    <bool>: true

Title: Check that we can ping localhost2 successfully
Meta:
    remedy.1: Check if DNS test failed first
    remedy.2: Check firewall rules
Command: ping_localhost2: exit-status:
Expected
    <int>: 2
to equal
    <int>: 0

Total Duration: 0.036s
Count: 2, Failed: 2, Skipped: 0
mikesir87 commented 4 years ago

We just ran into a use case for this kind of feature. We have a container image (so using dgoss) that we want to validate various console outputs when making HTTP requests. So, we'd like to do various http tests and then perform file tests against the console output.

aelsabbahy commented 4 years ago

@mikesir87 For that use case, the http request should be done outside of goss. Goss is not intended to change system state only read/view it.

The http checker is there to validate connectivity. It seems you're trying to use it to both validate connectivity and to trigger a log file change. The later is not a supported use case.

mikesir87 commented 4 years ago

Thanks for the feedback @aelsabbahy. It's too bad that's an unsupported use case because there is no other framework out there (that I've been able to find) that would make it as easy as this one. With the sequencing/ordering capability, it would make this an easy knockout. Anywho... thanks for the framework and happy holidays! ⛄️

aelsabbahy commented 4 years ago

No problem, sorry goss isn't able to meet this specific need.

EDIT: completely misread your comment I apologize for that, modified initial response.

mikesir87 commented 4 years ago

The example I was building was testing an Apache image that has various modsecurity rules configured. We wanted to start up the container (using dgoss), validate it's listening, and then perform various modsecurity tests. I was able to use http tests to trigger various endpoints and validate status codes/bodies. But, we wanted to validate a message appears in the container's stderr. So, I used the file test to check /goss/docker_output.log. But, then ran into ordering/concurrency. Setting the max concurrency to 1 fixes that, but the file test sometimes ran before the http test... which obviously doesn't work.

EDIT: And just noticed you modified your response too! 😆

aelsabbahy commented 4 years ago

Wonder if it would be helpful to support a PRE_GOSS_CMD option in dgoss. Allowing the user to arbitrarily put anything in there? Some thing that runs between goss_wait and actual goss test.

mikesir87 commented 4 years ago

Ah! That’s certainly an idea and would work for our use case. Then we can out the http tests there and the file ones in the next.

Only thought might be to have the ability to specify multiple goss test files, rather than just a PRE. For each file, just run goss and let it go. Could allow for other use cases where there might be more steps than what I have. Thoughts?

aelsabbahy commented 4 years ago

Leaning towards leaving it as a generic command. This provides flexibility for the user to chain however they like and stays out the way.

Multiple goss file support would add confusion in my opinion:

mikesir87 commented 4 years ago

I’m not sure I follow. Is there a built-in mechanism for gossfiles? I don’t see anything in the docs that suggests the ability to use multiple files.

aelsabbahy commented 4 years ago

https://github.com/aelsabbahy/goss/blob/master/docs/manual.md#gossfile

Was referring to this.

mikesir87 commented 4 years ago

Gotcha. Then I'm up for whatever you feel like you'd be willing to support. 😄And if it matters, I think the multiple file support I'm looking for would only needed for dgoss, not goss itself. For dgoss, it could simply just loop through multiple files and call goss on each. If you feel the usage of PRE_GOSS_CMD would work, I can make that work too.

And I know I've now highjacked the original request (sorry @sshipway!), so if you'd like to spin this into a separate issue, fine with me.

sshipway commented 4 years ago

@mikesir87 -- We have one test here that injects an email into the system, then another that checks the mailbox for the existence of said message and deletes it. Since goss doesn't allow test precendence, I simply wrapped these both into a single test script and called it as a command resource from goss instead. Simple tests could be done as an in-line script using command resource.

ilyaresh commented 4 years ago

The use case for me was :

  1. run command syslog event generator that will send sample event to local syslog listener which in turn will write the output to a file in a specified location (the path is dynamic and depends on command parameters and syslog configuration)
  2. validate that the file was created.

As you can see these had to be executed in the above mentioned order. Since anyway I was running the goss tests using Ansible, I ended up using multiple goss files and Ansible "controlling" the execution order. Full details here: http://isbyr.com/using-goss-and-ansible-templates-for-system-testing/

karimiehsan90 commented 4 years ago

@aelsabbahy Hi. My usecase is a build system, that has three phases, build, test, and deploy. Suppose that, the build system in build phase, generates a file named a.txt , in test phase, verifies content of a.txt, and in deploy phase, copies to production server. Now I add three tests, in my goss.yaml. But with no precedences, It may run test phase before build phase, and mark my test as failure. I solved this problem by adding this command in goss.yaml: build-system build && build-system test && build-system deploy But I'm not happy with this trick, because it is bad smell in my goss.yaml. Please add this feature to goss

aelsabbahy commented 4 years ago

I'm not a huge fan of goss changing system state, which seems to be the main driver for this feature request. That said..

There's quit a bit of discussion here. So I'm curious, if this was in goss what is the desired syntax that would meet everyone's need?

karimiehsan90 commented 4 years ago

I suggest two ways for it.

  1. Add a environment variable, for example named GOSS_TASKS_PARALLEL, that user can select how many tests run in parallel.
  2. Add a field named dependencies to each test, that can refer to another test. This ways satisfies my usecase. As I am java developer, in java, tests run in way No 1, which this value set to 1.
aelsabbahy commented 4 years ago

The first one already exists, see --max-concurrent flag.

However, I don't believe the order is deterministic. It would be a quick fix to sort to the result of Resources(). That way --max-concurrent 1 would be predictable: https://github.com/aelsabbahy/goss/blob/master/goss_config.go#L113

sshipway commented 4 years ago

I agree that goss is a testing system rather than a deployment system, and so shouldn't change the system (possibly changing test data though?) If you want to deploy, then maybe Puppet of similar would be a better fit for requirements.

However having said that, I would still like to have a precedence order for execution; some tests may conflict with each other, or require data to be set up first. You can do that with a script, but it is not so neat.

Ideally, for me, I would like an optional test attribute 'after' ( or 'requires') that specifies a certain test must come after another specified test. Execution of that test is delayed until the required tests have completed. If you wanted to be really fancy, you could have 'after' just specifying that the test has to have completed, and 'requires' saying it has to have completed successfully... You may also need a 'maxwait' setting to say how log a step is prepared to wait for its requirements to be met.

For example, two exec tests, with a specified order:

exec:
  foo:
    command: /bin/foo
  bar:
    command: /bin/bar
    after: exec:foo

The big problem with this, of course, is that you have to detect circular dependencies. The easiest way to do this would probably be a validation step taken after parsing and before tests start. Having a maxwait setting for a max time a step can wait for dependencies would avoid any risk of deadlock from circular dependencies, too.

ilyaresh commented 4 years ago

How about all the tests will have a default order of 100. If you want a specific test to run before or after others you charge its order value

sshipway commented 4 years ago

The problem with that (giving every test an order attribute), is that it would prevent executing tests in parallel with multiple threads (when order is not important) and so would result in a less efficient running of tests.

ilyaresh commented 4 years ago

I am not familiar with the actual code behind goss, but maybe when the test are loaded they can be split in "bins" based on the order values, and then when executed each "bin" is executed sequentially while all the test in a "bin" can be executed in parallel

sshipway commented 4 years ago

That would be the same as just running goss multiple times with difference config files -- in other words, defining 'stages' and picking which one each tests runs. I guess it would work, with all tests defaulting to stage 100 (for example) and then selected tests being given specific stages, but it seems a bit half-hearted if you understand what I mean.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

aelsabbahy commented 4 years ago

I'm not opposed to someone submitting the ordering that @sshipway is suggesting.

If someone is interested in taking this work on, feel free to either re-open this issue open a new issue with the following:

  1. DAG based test runner
  2. After attribute
  3. Require attribute
sshipway commented 4 years ago

I'd love to be able to work on this but I just don't have enough time to commit. I haven't even been able to finish the task of extracting test namevars out on every test type.

aelsabbahy commented 4 years ago

I've re-opened the issue. However, please comment on your thoughts for implementing this before doing so. I would like to discuss implementation details prior to work being done on the test runner.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.