Skip to content

Conversation

@star-szr
Copy link
Contributor

After reviewing the test and the code in lua/neogit/status.lua I noticed that stage and discard set M.current_operation but unstage does not.

The assertion for unstaging that occasionally fails comes right after an act() line, and the last line in act() is status.wait_on_current_operation(), which checks if status.current_operation is set.

Local testing results

I tested locally with just the four unstage tests (the other tests in the status spec were commented), and without this change I ran the test in a loop three times and it failed at runs 122, 119, and 74. With the fix in place, I was not able to reproduce a failure. I ran the test in a loop two times and stopped the loop at runs 1130 and 1360, satisfied that the random failure has been squashed.

Fixes: #887

Copy link
Member

@ten3roberts ten3roberts left a comment

Choose a reason for hiding this comment

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

The all too familar one line bugfix.

Thanks for your search and contribution.

Could you use the newer operation wrapper instead, which makes sure to reset in in the end too?

M.shared = operation("ignore_shared", function(popup)

Because I am not sure if it resets on every branch it to false/nil at the end

Copy link
Member

@ten3roberts ten3roberts left a comment

Choose a reason for hiding this comment

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

My bad, I see they are two different albeit similar APIs, old vs new.

The current is good, and it is reset without any apparent early returns that are missed if you expand the whole function 😄

Merging these two wait APIs are definitely out of scope for this PR 😅

@ten3roberts ten3roberts merged commit 0bb34b5 into NeogitOrg:master Oct 14, 2023
@star-szr star-szr deleted the 887-unstage-random-test-fail branch October 14, 2023 12:57
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.

Random test failure in tests/specs/neogit/status_spec.lua

2 participants