Skip to content

Comments

Test DagFileProcessorManager directly, not via the JobRunner#44642

Merged
ashb merged 1 commit intomainfrom
dagprocessor-test-reorg
Dec 4, 2024
Merged

Test DagFileProcessorManager directly, not via the JobRunner#44642
ashb merged 1 commit intomainfrom
dagprocessor-test-reorg

Conversation

@ashb
Copy link
Member

@ashb ashb commented Dec 4, 2024

90% of these tests created a DagProcessorJobRunner with the Manager inside it,
then did absolutely nothing with the JobRunner object. This makes the tests
more directly use what they are testing. (The rest of the time already created
the Manager directly)

(DagProcessorJobRunner itself is as simple as can be -- it calls start() ->
terminate() -> end() so we don't loose much of anything by not testing it
explicitly)

90% of these tests created a DagProcessorJobRunner with the Manager inside it,
then did absolutely nothing with the JobRunner object. This makes the tests
more directly use what they are testing.

(DagProcessorJobRunner itself is as simple as can be -- it calls `start()` ->
`terminate()` -> `end()` so we don't loose much of anything by not testing it
explicitly)
Copy link
Member Author

@ashb ashb left a comment

Choose a reason for hiding this comment

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

90% of the diff here is changingmanager.processor to manager and manager = DagProcessorJobRunner( to manager = DagProcessorManager(

This is a tidy-up PR before I work on these tests to swap them over to use the TaskSDK for parsing.

@ashb ashb merged commit 9bd1c40 into main Dec 4, 2024
@ashb ashb deleted the dagprocessor-test-reorg branch December 4, 2024 11:32
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
…44642)

90% of these tests created a DagProcessorJobRunner with the Manager inside it,
then did absolutely nothing with the JobRunner object. This makes the tests
more directly use what they are testing.

(DagProcessorJobRunner itself is as simple as can be -- it calls `start()` ->
`terminate()` -> `end()` so we don't loose much of anything by not testing it
explicitly)
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.

2 participants