Robottelo: Add @requires decorator to check available infrasctructure

[rochacbruno]

Tests should be marked with its depending infrastructure. e.g:

@requires("docker", "ldap")
def test_something_related_to_docker_and_ldap():
...

So if some of the requirements are not ok, the test case should be skipped or errored (but not count as failure).

That is specially useful for standalone automation when we have systems without all required infra.

the @requires should be modular taking keys to the validation functions for each infrastructure dependency allowing add of new requirements in an easy way,

[abalakh]

What's the difference between @requires and @skip_if_not_set decorators? Or you're just proposing to rename @skip_if_not_set to @requires?

[rochacbruno]

@abalakh skip_if_not_set is just verifying if there is a variable or session in properties file.

I am proposing that requires will really verify if that infrastructure is available, not by settings.

e.g: If there is a test requiring docker the requires should run a docker info command to check if docker is available and running.

For each requirement should be a command to check if it is healthy and running.

[abalakh]

@rochacbruno thanks for clarification

[elyezer]

I don't like the idea, if the infrastructure is not fully functional then the tests will fail because that. This seems just extra work that at the end will fail or skip tests, the current approach already does that. Also a failure is louder than a skip, 99.9% of the time we are looking on failed vs passed tests and not really caring about the skips at all.

The @skip_if_not_set transfers the responsibility to the person who is configuring Robottelo so Robottelo does not need to worry about every single piece of infrastructure it may require.

[elyezer]

I think a better documentation explaining the infrastructure setup will be a good thing to work on though. Currently we don't explain what is needed in order to the VirtualMachine from the robottelo.vm module work properly, for example.

[rochacbruno]

Just to keep this example together with the issue.

@requires('puppet', 'docker', 'ostree')
class SmartClassParametersTestCase(APITestCase):
    ....

then, in requires decorator we store a dict of validators

{'puppet': a_function_that_validates_presence_of_puppet, ...

If some of the requires returns False or Exception, the entire class is "Errored" or "Skipped" but can't count as failure as tests did not run.

[elyezer]

Any exception raised during the test execution will be counted as a failure unless the exception is handled and not reraised.

All the Robottelo's decorators are lazy and evaluated at run time rather than import time since doing a ssh connection during import time causes some dead lock. Have this in mind when working on this issue to avoid automation stuck.

[sghai]

@rochacbruno : Is this still valid ? or can we close this ?

[rochacbruno]

@sghai we have not discussed deeply about this issue, also I think my idea have been misunderstood.

I think a test can end in 4 statuses

1. Passed - Infra worked, Test passed
2. Failed - Infra worked, Test failed
3. Errored - Infra doen't work (test not even executed)
4. Skipped - Not executed for some good reason (i.e: bz_open, not_set, marks, etc..)

That diference in status helps when analysing reports, currently, for example if we are running a case from test_docker and lets say docker.service is not installed or not working for some reason, the test status would simply "Fail" counting in our reports as our test is not working, but the test is not even executed.

If we can separate those statuses we can see exactly how much of our problems are related to infrastructure.

I still think it would be useful if we can check which tests can run based on infra/settings requirements.

But maybe I am wrong, so I call for a round of discussion around this when we have some time for this,

Cc: @omaciel

[rplevka]

@rochacbruno

Failed - Infra worked, Test failed
Errored - Infra doen't work (test not even executed)

This is how it already works. the test is being marked as FAILED only if the assertions are not met. Any other exception leads to test being marked as ERROR.

[rochacbruno]

There are some tests (in UI) with no assertions, So the absence of error makes it to be a "passed" if something wrong by lack of infrastructure it will raise Exception which means error, but counts as fail.

But I have to gather more information about this, so I'll keep this open until I have more data (if I am wrong so I'll close this)

[elyezer]

The unittest makes a clear distinction between an error and a failure. What @rplevka mentioned is what really happen.

For the unittest any exception other than AssertionError is counted as an error while AssertionError is counted as a failure.

We can give the meaning we want. For example we may decide that any non expected exception will be a infrastructure issue. But the current state of Robottelo does not have a clear distinction about errors and failures.

That said, if we decide to make a clear distinction between errors and failures, then we will need to do a great refactor. To Jenkins everything is a failure so even if we refactor we will need to look on the pytest output or the source jUnit XML report to check that. My opinion is that I don't see a good reason to make a clear distinction.

An example is the CLI commands that raise a custom exception when the return code is not zero. So that is a failure (because the system behaved unexpectedly) but unittest will consider it being an error.

[renzon]

@elyezer thanks for the explanation of decorators lazy creation. I have tried made skip_if_os in import time but was getting an error. I just saw other decorators and made it lazy, but didn't know the reason at the time.