Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests calling workflow are not independent, modifying resources key in one test impacts another test #1148

Open
1 of 5 tasks
mieczyslaw opened this issue Aug 12, 2024 · 0 comments

Comments

@mieczyslaw
Copy link

Pre-bug-report checklist

1. This bug can be reproduced using pure Argo YAML

If yes, it is more likely to be an Argo bug unrelated to Hera. Please double check before submitting an issue to Hera.

2. This bug occurs in Hera when...

  • exporting to YAML
  • submitting to Argo
  • running on Argo with the Hera runner
  • [X ] other: running unit tests when there is more than one unit test in the module and one unit test modifies the workflow object

Bug report

Describe the bug
I have the simple hello-world workflow. For users I support I want to verify that resources are always set. I wrote unit tests - one test (a simplified version as the original one checks whether exception is raised on content validation with resources being None) sets resources key to None, and the other test checks whether resources are not None.

When run separately, both tests pass, but when run as a module, the test which sets resources to None causes another test to fail (in practice I have a few ore tests and all fail after the one setting resources to None). It looks like there is no fresh workflow assigned in every test even though I call the workflow and assign to wf. However, I checked that wf and templates IDs are different in both tests so it is not a kind of object reuse issue.

When I do import in every test body separately (so no import in the beginning of the module), it does NOT help. However I discovered that copy.deepcopy() helps. In fact in this case it is enough to call deepcopy in the test modifying the template to make all running. Calling copy.deepcopy() only on the second test (the one which does not modify the resources) does NOT help neither. If you modify the order so the test setting resources=None is the last one in the module, all passes.

My only explanation after all I checked so far is that somehow script decorator is not executed again?

To Reproduce
Workflow module:

import hera.workflows as hwf


@hwf.script(
    resources=hwf.Resources(
        cpu_request=1, cpu_limit=1, memory_request="1Gi", memory_limit="1Gi"
    )
)
def say_hello() -> None:
    print("Hello, world!")


def hello_world_example_wf() -> hwf.Workflow:
    workflow_entrypoint = "hello-steps"
    with hwf.Workflow(
        generate_name="hello-world-",
        archive_logs=True,
        entrypoint=workflow_entrypoint,
        labels={"workflow_name": "hello-world-example"},
    ) as wf, hwf.Steps(name=workflow_entrypoint):
        _hello_step = say_hello()  # type:ignore[call-arg]

    return wf

Unit tests module:

import copy
from hello_world_example import hello_world_example_wf


def test_workflow_missing_resources() -> None:
    wf = hello_world_example_wf()
    # wf = copy.deepcopy(hello_world_example_wf())

    # remove resources key as no resources set at all
    for tmpl in wf.templates:
        if hasattr(tmpl, "resources"):
            tmpl.resources = None

    for tmpl in wf.templates:
        if hasattr(tmpl, "resources"):
            assert tmpl.resources is None


def test_workflow_resources_present() -> None:
    wf = hello_world_example_wf()
    # wf = copy.deepcopy(hello_world_example_wf())

    for tmpl in wf.templates:
        if hasattr(tmpl, "resources"):
            assert tmpl.resources is not None

The second tests fails (without copy.deepcopy) with:

def test_workflow_resources_present() -> None:
        wf = hello_world_example_wf()
        # wf = copy.deepcopy(hello_world_example_wf())
    
        for tmpl in wf.templates:
            if hasattr(tmpl, "resources"):
>               assert tmpl.resources is not None
E               AssertionError: assert None is not None

Expected behavior
Tests should be independent and modifying templates (or any other workflow key) in one test shouldn't impact other tests, it shouldn't be necessary to call copy.deepcopy().

Environment

  • Hera Version: 5.16.1
  • Python Version: 3.11.9
  • Argo Version: 3.5.8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant