Skip to content

added additional test to the pse distribute method#16

Merged
miladz68 merged 7 commits into
masterfrom
milad/distribute-pse-continued
Nov 13, 2025
Merged

added additional test to the pse distribute method#16
miladz68 merged 7 commits into
masterfrom
milad/distribute-pse-continued

Conversation

@miladz68

@miladz68 miladz68 commented Nov 10, 2025

Copy link
Copy Markdown
Contributor

Description

Reviewers checklist:

  • Try to write more meaningful comments with clear actions to be taken.
  • Nit-picking should be unblocking. Focus on core issues.

Authors checklist

  • Provide a concise and meaningful description
  • Review the code yourself first, before making the PR.
  • Annotate your PR in places that require explanation.
  • Think and try to split the PR to smaller PR if it is big.

This change is Reviewable

@miladz68 miladz68 requested a review from a team as a code owner November 10, 2025 15:17
@miladz68 miladz68 requested review from TxCorpi0x, masihyeganeh and ysv and removed request for a team November 10, 2025 15:17

@TxCorpi0x TxCorpi0x left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @masihyeganeh and @ysv)


x/pse/keeper/distribute_test.go line 136 at r1 (raw file):

				func(r *runEnv) { assertScoreResetAction(r) },
			},
		},
  1. We can have a test like test distribution with zero total, all should go to community pool?
  2. As well as test multiple distributions without delegation changes to get rewards for all periods
  3. Account the undelegated a few of delegations, some of them gone and some of them remain
  4. Or edge case, account that undelegated right before distribution, (would it be a race condition?

x/pse/keeper/hooks_test.go line 293 at r1 (raw file):

		})
	r.requireT.NoError(err)
	r.requireT.Equal(0, count)

This line is checking the count of AccountScoreSnapshot again.

@miladz68 miladz68 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @masihyeganeh, @TxCorpi0x, and @ysv)


x/pse/keeper/distribute_test.go line 136 at r1 (raw file):

Previously, TxCorpi0x wrote…
  1. We can have a test like test distribution with zero total, all should go to community pool?
  2. As well as test multiple distributions without delegation changes to get rewards for all periods
  3. Account the undelegated a few of delegations, some of them gone and some of them remain
  4. Or edge case, account that undelegated right before distribution, (would it be a race condition?
  1. Done.
  2. Done.
  3. exists
  4. exists

x/pse/keeper/hooks_test.go line 293 at r1 (raw file):

Previously, TxCorpi0x wrote…

This line is checking the count of AccountScoreSnapshot again.

Done.

@TxCorpi0x TxCorpi0x left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @masihyeganeh and @ysv)

@ysv ysv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ysv reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @masihyeganeh)

@miladz68 miladz68 merged commit 4eb0c0b into master Nov 13, 2025
14 of 16 checks passed
@miladz68 miladz68 deleted the milad/distribute-pse-continued branch November 13, 2025 09:18
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