Skip to content

Conversation

@nickrobinson251
Copy link
Member

@nickrobinson251 nickrobinson251 commented Nov 1, 2025

We've had feedback from users that this option is really helpful, so let's make it the default.

Here's a data point for the overhead it adds for 1,000 testitems:

julia> for fileno in 1:100
           fname = "tmp/file$(fileno)_test.jl"
           open(fname, "w") do io
               for i in 1:100
                   tnum = (fileno-1)*100 + i
                   write(io, """
                       @testitem "$tnum" begin
                           @test $(rand(Bool))
                       end
                       """
                   )
               end
           end
       end

julia> runtests("tmp/") # run once to populate test statuses

julia> runtests("tmp/") # run again with the additional cost of sorting testitems
...
DEBUG @ ReTestItems.jl:429 | Sorted test items with failures first in 0.00694 seconds.
...

where i added this timing info like:

--- a/src/ReTestItems.jl
+++ b/src/ReTestItems.jl
@@ -420,10 +420,13 @@ function _runtests_in_current_env(
         (nworkers == 0 ? "" : " with $nworkers worker processes and $nworker_threads threads per worker.")
     try
         if cfg.failures_first && !isempty(GLOBAL_TEST_STATUS)
+            t0 = time()
             sort!(testitems.testitems; by=_status_when_last_seen)
             foreach(enumerate(testitems.testitems)) do (i, ti)
                 ti.number[] = i # reset number to match new order
             end
+            tt = time() - t0
+            @debugv 2 "Sorted test items with failures first in $(round(tt, digits=5)) seconds."
             is_sorted_queue = true

I added ReTestItems.reset_test_status!() to tests as needed. This isn't public API but now failures_first=true is the default i think more likely we will get a feature request for it -- let's see.

@nickrobinson251 nickrobinson251 marked this pull request as ready for review November 4, 2025 15:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes the default value of failures_first from false to true, making ReTestItems run previously failed test items first by default. This is a minor version bump (v1.35.0) that changes default behavior to optimize the development workflow.

  • Changed default value of failures_first parameter from false to true
  • Updated documentation to reflect the new default behavior
  • Added test status resets to ensure test isolation

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
Project.toml Version bump to 1.35.0 for this feature change
src/ReTestItems.jl Updated failures_first default value to true in function signature and docstring
README.md Updated documentation to explain the new default behavior and when it was introduced
test/integrationtests.jl Added reset_test_status!() calls to prevent test interference from new default behavior
Comments suppressed due to low confidence (1)

test/integrationtests.jl:1246

  • The test expects test item 'no skip, 1 pass' to run first (1/6), but with failures_first=true now being the default, this test could fail if the same file was run in a previous test and had failures, causing different ordering. Consider explicitly passing failures_first=false to runtests to ensure the test verifies the original ordering behavior it was designed to test.
    @test contains(c1.output, r"START \(1/6\) test item \"no skip, 1 pass\"")

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@NHDaly NHDaly left a comment

Choose a reason for hiding this comment

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

❤️ This is turning into such a great little testing framework ❤️

@nickrobinson251 nickrobinson251 merged commit 934912f into main Nov 5, 2025
12 of 15 checks passed
@nickrobinson251 nickrobinson251 deleted the npr-failures-first-default branch November 5, 2025 07:40
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.

3 participants