diff mbox series

[autotest] UBUNTU: SAUCE: fix missing setup on second run

Message ID 20210603132118.40298-1-krzysztof.kozlowski@canonical.com
State New
Headers show
Series [autotest] UBUNTU: SAUCE: fix missing setup on second run | expand

Commit Message

Krzysztof Kozlowski June 3, 2021, 1:21 p.m. UTC
The autotest had nice idea that test setup can be executed only once on
a machine, till test version does not change.  If version stays the
same, autotest will not run test setup on next executions.

However this requirement/feature was not defined clear enough and some
tests:
1. Perform cleanup() which reverts the setup() (e.g. ubuntu_boot).
2. Set runtime configuration (e.g. initialize object's field in Python
   __init__()) during setup() and read it on each test (e.g.
   ubuntu_sysdig_smoke_test).

All of these cases fail.  Fix this by calling the test setup() every
time.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
---
 client/shared/utils.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Sean Feole June 3, 2021, 1:42 p.m. UTC | #1
This will not have any negative affect on the way SRU testing operates.  
I agree that it is annoying setup() will only get run once, unless you 
iterate the version +=1. It's also not apparent to someone unless they 
rummage through the autotest docs.

Unfortunately, not every test in our collection operate in the same 
manner, there are some that do not use a setup() but that is neither 
here nor there. I have also just modified the version if running through 
any sort of triage in the past. This is because the intended use case 
always runs once on a freshly deployed system.

This is good Krzysztof!   Will def help for those working on test 
improvements.

+1 from me.

-Sean


On 6/3/21 9:21 AM, Krzysztof Kozlowski wrote:
> The autotest had nice idea that test setup can be executed only once on
> a machine, till test version does not change.  If version stays the
> same, autotest will not run test setup on next executions.
>
> However this requirement/feature was not defined clear enough and some
> tests:
> 1. Perform cleanup() which reverts the setup() (e.g. ubuntu_boot).
> 2. Set runtime configuration (e.g. initialize object's field in Python
>     __init__()) during setup() and read it on each test (e.g.
>     ubuntu_sysdig_smoke_test).
>
> All of these cases fail.  Fix this by calling the test setup() every
> time.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>   client/shared/utils.py | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/client/shared/utils.py b/client/shared/utils.py
> index 3e8c9448d23e..367edcd04a03 100644
> --- a/client/shared/utils.py
> +++ b/client/shared/utils.py
> @@ -862,7 +862,11 @@ def update_version(srcdir, preserve_srcdir, new_version, install,
>                                        os.path.basename(patch_src))
>               shutil.copyfile(patch_src, patch_dst)
>   
> -        install(*args, **dargs)
> +    # The test-specific setup is always needed because:
> +    #  - it could have been reverted with cleanup()
> +    #  - few broken tests might depend on setup() run always
> +    install(*args, **dargs)
> +    if install_needed:
>           pickle.dump(new_version, open(versionfile, 'w'))
>   
>
Sean Feole June 3, 2021, 2:19 p.m. UTC | #2
Acked-by Sean Feole <sean.feole@canonical.com>

On 6/3/21 9:21 AM, Krzysztof Kozlowski wrote:
> The autotest had nice idea that test setup can be executed only once on
> a machine, till test version does not change.  If version stays the
> same, autotest will not run test setup on next executions.
>
> However this requirement/feature was not defined clear enough and some
> tests:
> 1. Perform cleanup() which reverts the setup() (e.g. ubuntu_boot).
> 2. Set runtime configuration (e.g. initialize object's field in Python
>     __init__()) during setup() and read it on each test (e.g.
>     ubuntu_sysdig_smoke_test).
>
> All of these cases fail.  Fix this by calling the test setup() every
> time.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
>   client/shared/utils.py | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/client/shared/utils.py b/client/shared/utils.py
> index 3e8c9448d23e..367edcd04a03 100644
> --- a/client/shared/utils.py
> +++ b/client/shared/utils.py
> @@ -862,7 +862,11 @@ def update_version(srcdir, preserve_srcdir, new_version, install,
>                                        os.path.basename(patch_src))
>               shutil.copyfile(patch_src, patch_dst)
>   
> -        install(*args, **dargs)
> +    # The test-specific setup is always needed because:
> +    #  - it could have been reverted with cleanup()
> +    #  - few broken tests might depend on setup() run always
> +    install(*args, **dargs)
> +    if install_needed:
>           pickle.dump(new_version, open(versionfile, 'w'))
>   
>
diff mbox series

Patch

diff --git a/client/shared/utils.py b/client/shared/utils.py
index 3e8c9448d23e..367edcd04a03 100644
--- a/client/shared/utils.py
+++ b/client/shared/utils.py
@@ -862,7 +862,11 @@  def update_version(srcdir, preserve_srcdir, new_version, install,
                                      os.path.basename(patch_src))
             shutil.copyfile(patch_src, patch_dst)
 
-        install(*args, **dargs)
+    # The test-specific setup is always needed because:
+    #  - it could have been reverted with cleanup()
+    #  - few broken tests might depend on setup() run always
+    install(*args, **dargs)
+    if install_needed:
         pickle.dump(new_version, open(versionfile, 'w'))