+1

...and the same could be used in filter_component_builds. I would suggest extracting the common piece of code into a separate _order_by function.

@ncoghlan I'm good with this change. I'm just warning you that a lot of the tests will need fixing because they are testing the API assuming it is ordered by ID from oldest to newest.

:+1: here too.

https://pagure.io/fm-orchestrator/pull-request/748 is the first cut at a PR. Assuming the vcrpy files correctly invalidated themselves (which may be a bad assumption), only two tests appear to have been specifically relying on the default API response ordering (marked with comments in the PR).

It wasn't immediately obvious to me what the right fix for that was, since the test suite seems to allow arbitrary additional entries to build up in the test DB during execution, which would mean we couldn't just change those tests to expect the results they're now getting (since they may change over time).

since the test suite seems to allow arbitrary additional entries to build up in the test DB during execution

This is a surprise to me. We should tear it down.. with tearDown().

@ncoghlan in the setUp function, init_data() is called which deletes the database entirely and adds the default entries back in.

@mprahl Thanks, I'd managed to miss that.

In that case, my next question would be whether or not it's OK for these tests to assume that test.init_data won't be expanded to add more records.

That feels like a fairly fragile assumption to me, so it may still be worth following the link through to the "oldest" page of results, since that's likely to be more stable (I'd expect changes to init_data to be more likely to involve appending to it, rather than inserting code at the start).

@ncoghlan yes it is fragile, but it hasn't changed in a while, and if we change it, we know we'll have to fix the unit tests. With that said, for the time being, you can just replicate what's already there.

Rebased and updated PR pushed. After rebasing, I saw that "test_query_builds" and "test_query_builds_non_verbose" were now checking the same thing, so I just consolidated them into a single test.

This issue has been migrated to Fedora Forge:
https://forge.fedoraproject.org/koji/mbs/issues/739

Please continue any further discussion there.