Skip to content

box: cover box_wait limbo_acked#9234

Merged
sergepetrenko merged 1 commit intotarantool:masterfrom
Astronomax:master
Nov 2, 2023
Merged

box: cover box_wait limbo_acked#9234
sergepetrenko merged 1 commit intotarantool:masterfrom
Astronomax:master

Conversation

@Astronomax
Copy link
Contributor

Prior to this patch, there were many possible code execution options
that were not covered by tests. After this commit, any assert(false)
inside box_wait_limbo_acked cause a crash.

Closes #7318

NO_DOC=test
NO_CHANGELOG=test

@coveralls
Copy link

coveralls commented Oct 7, 2023

Coverage Status

coverage: 86.394% (+0.04%) from 86.353% when pulling 9b5aa3a on Astronomax:master into a9d07e9
on tarantool:master
.

@sergepetrenko sergepetrenko added the coverage temporary label, ask @NickVolynkin label Oct 10, 2023
Copy link
Collaborator

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

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

Good job on the patch!

Please find my comments below.

@sergepetrenko sergepetrenko removed the coverage temporary label, ask @NickVolynkin label Oct 10, 2023
Copy link
Collaborator

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

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

Some more comments from me

@Astronomax Astronomax force-pushed the master branch 7 times, most recently from 9dccafa to 61282db Compare October 24, 2023 09:42
@sergepetrenko sergepetrenko self-assigned this Oct 24, 2023
Copy link
Collaborator

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes!

Looks nice. I only have a couple comments left.

Copy link
Collaborator

@sergepetrenko sergepetrenko left a comment

Choose a reason for hiding this comment

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

Good job! LGTM.

Next time please attach a diff in a comment when changes are minor. It's much easier to re-check the patch this way.

Copy link
Collaborator

@Gerold103 Gerold103 left a comment

Choose a reason for hiding this comment

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

Thanks for the patch!

@Astronomax Astronomax force-pushed the master branch 3 times, most recently from 7f91ab2 to 5da8500 Compare October 26, 2023 17:30
@Astronomax Astronomax force-pushed the master branch 2 times, most recently from 648f490 to 8791d15 Compare October 26, 2023 17:49
@Astronomax Astronomax requested a review from Gerold103 October 27, 2023 07:24
@Astronomax Astronomax force-pushed the master branch 2 times, most recently from a2525d9 to 96ddd91 Compare October 31, 2023 13:08
Prior to this patch, there were many possible code execution options
that were not covered by tests. After this commit, any assert(false)
inside box_wait_limbo_acked cause a crash.

Closes tarantool#7318

NO_DOC=test
NO_CHANGELOG=test
Copy link
Collaborator

@Gerold103 Gerold103 left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes!

@sergepetrenko sergepetrenko added the full-ci Enables all tests for a pull request label Nov 1, 2023
@sergepetrenko sergepetrenko merged commit 7fce5be into tarantool:master Nov 2, 2023
@sergepetrenko
Copy link
Collaborator

Cherry-picked to 2.11: a44ed8c
Will cherry-pick to 2.10 in scope of #9327

@Astronomax Astronomax changed the title box: cover box_wait limbo_acked box: cover box_wait limbo_acked Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci Enables all tests for a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cover box_wait_limbo_acked with tests

6 participants