Fix relative ordering in a complex transitive cases#242
Conversation
apply_relative_constraints() unconditionally rebuilt sorted_list as start_pinned + sorted_middle + end_pinned, discarding the unordered items that sort_numbered_items() had interspersed between numbered items in sparse ordering mode. Early-return when there are no relative or dependency marks to preserve the layout produced by sort_numbered_items(). Fixes 5 sparse ordering tests.
for more information, see https://pre-commit.ci
| """ | ||
| ) | ||
| result = test_path.runpytest("-v", "--error-on-failed-ordering") | ||
| if int(pytest.__version__.split(".")[0]) < 6: |
There was a problem hiding this comment.
This looks like a special case for some past version. Does it make sense to preserve it?
There was a problem hiding this comment.
Sure, we don't support pytest < 6 anymore.
| result.stdout.fnmatch_lines( | ||
| [ | ||
| "test_failed_ordering.py::test_2 PASSED", | ||
| "test_failed_ordering.py::test_1 ERROR", |
There was a problem hiding this comment.
Not sure what this tested and if it still makes sense.
There was a problem hiding this comment.
This was testing the --error-on-failed-ordering option, though the test used was one that failed due a shortcoming of the algorithm. Not sure if there still is the possibility of cyclic dependencies that cannot be resolved.
There was a problem hiding this comment.
I see, thanks. Why, there is always a possibility of a cyclic dependency, isn't there?
victor-nexthop
changed the title
|
Thanks! I will have a closer look at this, though this may take some time (just back from traveling). |
|
|
||
| pytest-order emits a WARNING for each such impossible constraint and preserves | ||
| the absolute ordinal positions unchanged. | ||
| """ |
There was a problem hiding this comment.
This changes the current behavior, which was intentional. Explicit dependencies between tests shall be prioritized over ordinal sorting - I still think that makes sense.
Can you elaborate why you want to change this?
There was a problem hiding this comment.
I was just in the mindset of "all ordering shall be satisfied", and didn't read the docs that deep. Wouldn't have made it a requirement.
There was a problem hiding this comment.
I think using topological sorting still makes sense?
I'll rework the changes according to the design decision, likely next week.
2 participants
We only use
beforeandafterconstraints, and sometimes, with sparse transitive dependencies,pytest-orderwon't produce the order that we expect, silently ignoring one of the constraints.This is a set of changes that replaces the current algorithm with a topological sort, and adds a warning about impossible sets of constraints, for example:
@order(first) def test_foo(): pass; @order(before="test_foo") def test_bar(): pass.Created mostly by Claude.
This is a conversation starter. Please let me know if it needs to be squashed and split into smaller commits.