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

DTT1 iteration 3 workflow engine unit tests #5072

Conversation

mhamra
Copy link
Contributor

@mhamra mhamra commented Mar 4, 2024

Related issue: #4996

Description

This PR implements the unit tests for DTT1 Workflow Engine Module

@mhamra mhamra linked an issue Mar 4, 2024 that may be closed by this pull request
3 tasks
@mhamra mhamra force-pushed the 4996-dtt1-iteration-3-workflow-engine-unit-tests branch 2 times, most recently from 2ff97e8 to a5a544e Compare March 8, 2024 15:00
Copy link
Member

@rauldpm rauldpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, check the comments, also, please consider adding documentation to each test case about the parameters and the expected assertion case

Please provide a test usage output

Copy link
Member

@rauldpm rauldpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, please check the review comment

Also, we need to validate if this is expected or not

============================================================================================= short test summary info =============================================================================================
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_set_status[task1-non_existing_status-dag0] - KeyError: 'non_existing_status'
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_set_status[non_existing_task-non_existing_status-dag0] - KeyError: 'non_existing_status'
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task1-abort-related-flows-to_be_canceled1-dag0] - AssertionError: assert {'task1', 'ta...sk4', 'task5'} == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task1-abort-related-flows-to_be_canceled1-dag1] - AssertionError: assert {'task1', 'ta...sk4', 'task5'} == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task1-continue-to_be_canceled2-dag0] - assert set() == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task1-continue-to_be_canceled2-dag1] - assert set() == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task2-abort-all-to_be_canceled3-dag0] - AssertionError: assert {'task1', 'ta...sk4', 'task5'} == {'task1'}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task2-abort-all-to_be_canceled3-dag1] - AssertionError: assert {'task1', 'ta...sk4', 'task5'} == {'task1'}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task2-abort-related-flows-to_be_canceled4-dag0] - AssertionError: assert {'task1', 'ta...sk4', 'task5'} == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task2-abort-related-flows-to_be_canceled4-dag1] - AssertionError: assert {'task1', 'ta...sk4', 'task5'} == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task2-continue-to_be_canceled5-dag0] - assert set() == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task2-continue-to_be_canceled5-dag1] - assert set() == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task5-continue-to_be_canceled8-dag0] - assert set() == {}
FAILED deployability/modules/workflow_engine/tests/test_dag.py::test_cancel_dependant_tasks[task5-continue-to_be_canceled8-dag1] - assert set() == {}
========================================================================================== 14 failed, 78 passed in 0.31s ==========================================================================================

Copy link
Member

@rauldpm rauldpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @mhamra, let's wait for the team review the pytest results

@mhamra
Copy link
Contributor Author

mhamra commented Apr 5, 2024

UT FAILURE UPDATE

In this commit, I adjusted all the failing test cases to pass with no failures. Please confirm the following modifications:

test_cancel_dependant_tasks

The test_dag_cancel_dependent_tasks UT tests the DAG.cancel_dependent_tasks method. The test creates a DAG instance with a dependency graph in the image. The DAG failed, canceled, or successful sets are empty.

The arrows in the graph in the image indicate that task2 depends on task1, and that task5 depends on task2, task3 and task4
The image table shows the parameters I used in the test cases and the results obtained when executing the method. I've adjusted the expected results to the actual behavior of the method. Are all the expected results okay?

image_720

test_set_status

The Unit Tests fail when the value of the status parameter is invalid (when it is not in ['failed', 'canceled', 'successful']). Should those test cases be removed, or should they be maintained?

rauldpm
rauldpm previously approved these changes Apr 5, 2024
Copy link
Member

@rauldpm rauldpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

= 89 passed in 0.27s =

@rauldpm rauldpm changed the base branch from 4.9.0 to enhancement/4495-DTT1 April 9, 2024 12:37
@rauldpm rauldpm changed the base branch from enhancement/4495-DTT1 to 4.9.0 April 9, 2024 12:38
@rauldpm rauldpm dismissed their stale review April 9, 2024 12:38

The base branch was changed.

@rauldpm rauldpm changed the base branch from 4.9.0 to enhancement/4495-DTT1 April 9, 2024 12:38
Copy link
Member

@rauldpm rauldpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrate commits to a new branch with origin enhancement/4495-DTT1)

@mhamra mhamra force-pushed the 4996-dtt1-iteration-3-workflow-engine-unit-tests branch from ee7ccd0 to da889f3 Compare April 9, 2024 15:07
@fcaffieri fcaffieri merged commit f7c5df4 into enhancement/4495-DTT1 Apr 9, 2024
1 check passed
@fcaffieri fcaffieri deleted the 4996-dtt1-iteration-3-workflow-engine-unit-tests branch April 9, 2024 19:43
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

Successfully merging this pull request may close these issues.

DTT1 - Iteration 3 - Unit test - Unit tests for the Workflow module
3 participants