Skip to content

Fix test in PR 12663 #12667

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.

Sign up for GitHub

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

Jump to bottom
Merged
kaxil merged 1 commit into from
Nov 27, 2020
Merged

Fix test in PR 12663 #12667

kaxil merged 1 commit into from
Nov 27, 2020

Conversation

@XD-DENG
Copy link
Member

@XD-DENG XD-DENG commented Nov 27, 2020

The order of the two TI keys in test_process_task_instances_depends_on_past() (#12663) is not guaranteed,
in such a case, directly assert two lists (both with two elements) will cause 'random' failure

This PR addresses this issue.

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@XD-DENG XD-DENG added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Nov 27, 2020
@XD-DENG XD-DENG added this to the Airflow 1.10.14 milestone Nov 27, 2020
@XD-DENG XD-DENG requested review from ashb and kaxil November 27, 2020 20:33
@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Nov 27, 2020
@XD-DENG
Copy link
Member Author

XD-DENG commented Nov 27, 2020

Feel free to drop this PR if any other better way should be favored.

@XD-DENG XD-DENG removed the area:Scheduler including HA (high availability) scheduler label Nov 27, 2020
XD-DENG
XD-DENG commented Nov 27, 2020
View reviewed changes
@github-actions
Copy link

github-actions bot commented Nov 27, 2020

The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run it!

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Nov 27, 2020
@XD-DENG
Copy link
Member Author

XD-DENG commented Nov 27, 2020

Force-pushed (without further change) to trigger full test. Just be on the safe side ;-)

kaxil
kaxil reviewed Nov 27, 2020
View reviewed changes
@XD-DENG @kaxil
Fix test in PR 12663
edb79a3 
The order of the two TI keys in test_process_task_instances_depends_on_past() is not guaranteed,
in such a case, directly assert two lists (both with two elements) will cause 'random' failure

This PR addresses this issue.

Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
potiuk
potiuk approved these changes Nov 27, 2020
View reviewed changes
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Oh yeah!

@XD-DENG
Copy link
Member Author

XD-DENG commented Nov 27, 2020

Three failures were observed, all are TestUndefinedJinjaVariablesRule::test_invalid_check. I notice it's existing in the latest a few commits in branch v1-10-stable. So I assume it's not related to this PR, and this PR should be good?

I believe you guys can help better judge given you are following up on v1-10 tightly? @kaxil @potiuk .

Thanks!

@kaxil kaxil merged commit 65a8dbc into apache:v1-10-stable Nov 27, 2020
@XD-DENG XD-DENG deleted the fix-ci branch November 27, 2020 22:47
@kaxil
Copy link
Member

kaxil commented Nov 27, 2020

Three failures were observed, all are TestUndefinedJinjaVariablesRule::test_invalid_check. I notice it's existing in the latest a few commits in branch v1-10-stable. So I assume it's not related to this PR, and this PR should be good?

I believe you guys can help better judge given you are following up on v1-10 tightly? @kaxil @potiuk .

Thanks!

Yup, that's fine. Will have to fix it separately, I think that is related to Py2.7 / Py3.5

kaxil added a commit that referenced this pull request Nov 30, 2020
@XD-DENG @kaxil
Fix test in PR 12663 (#12667)
52116d3 
The order of the two TI keys in test_process_task_instances_depends_on_past() is not guaranteed,
in such a case, directly assert two lists (both with two elements) will cause 'random' failure

This PR addresses this issue.

Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>

Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
@potiuk potiuk added the type:misc/internal Changelog: Misc changes that should appear in change log label Nov 30, 2020
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
@XD-DENG @kaxil
Fix test in PR 12663 (apache#12667)
8137719 
The order of the two TI keys in test_process_task_instances_depends_on_past() is not guaranteed,
in such a case, directly assert two lists (both with two elements) will cause 'random' failure

This PR addresses this issue.

Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>

Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaxil kaxil kaxil approved these changes

@potiuk potiuk potiuk approved these changes

@ashb ashb Awaiting requested review from ashb

Assignees

No one assigned

Labels

changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Milestone

Airflow 1.10.14

Development

Successfully merging this pull request may close these issues.

3 participants

@XD-DENG @kaxil @potiuk