test(mempool): rename get_txs test to be more specific

[ayeletstarkware]

This change is 

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.21%. Comparing base (93de0bd) to head (0a55844).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #485   +/-   ##
=======================================
  Coverage   81.21%   81.21%           
=======================================
  Files          42       42           
  Lines        1826     1826           
  Branches     1826     1826           
=======================================
  Hits         1483     1483           
  Misses        269      269           
  Partials       74       74           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[elintul]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware and @giladchase)

---

crates/mempool/src/mempool_test.rs line 143 at r1 (raw file):

#[case::test_get_more_than_all_eligible_txs(5)]
#[case::test_get_less_than_all_eligible_txs(2)]
fn test_get_txs_fetching_order(#[case] requested_txs: usize) {

Not sure it adds to readability, since this is checked in every get_txs tets, right?

Code quote:

test_get_txs_fetching_order

[ayeletstarkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)

---

crates/mempool/src/mempool_test.rs line 143 at r1 (raw file):

Previously, elintul (Elin) wrote…

Not sure it adds to readability, since this is checked in every get_txs tets, right?

not really. For example, when we want to fill a hole, we check that no txs are fetched.
Also, it is one of the splits @giladchase asked for, here #254

[elintul]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)

---

crates/mempool/src/mempool_test.rs line 143 at r1 (raw file):

Previously, ayeletstarkware (Ayelet Zilber) wrote…

not really. For example, when we want to fill a hole, we check that no txs are fetched.
Also, it is one of the splits @giladchase asked for, here #254

"when we want to fill a hole, we check that no txs are fetched"
Can you explain?

Isn't the case is that in all tests of get_txs we check the order of the returned transactions?

[ayeletstarkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)

---

crates/mempool/src/mempool_test.rs line 143 at r1 (raw file):

Previously, elintul (Elin) wrote…

"when we want to fill a hole, we check that no txs are fetched"
Can you explain?

Isn't the case is that in all tests of get_txs we check the order of the returned transactions?

In most tests, we focus on something else, but order. for example, only one tx is returned, so the order is not checked.

[elintul]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)

---

crates/mempool/src/mempool_test.rs line 143 at r1 (raw file):

Previously, ayeletstarkware (Ayelet Zilber) wrote…

In most tests, we focus on something else, but order. for example, only one tx is returned, so the order is not checked.

Right, it's trivial there. What about rest of get_txs tests?

[ayeletstarkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @giladchase)

---

crates/mempool/src/mempool_test.rs line 143 at r1 (raw file):

Previously, elintul (Elin) wrote…

Right, it's trivial there. What about rest of get_txs tests?

• test_get_txs - checks order by the tip.
• test_get_txs_multi_nonce - only one account, so doesn't check order by tip
• test_get_txs_with_holes_multiple_accounts - two txs of two accounts, one with a hole, so only one tx is returned. no order check
• test_get_txs_with_holes_single_account - single account, one tx, with a hole, nothing is returned, no order check.
• test_flow_filling_holes - flow of add_tx and get_txs.

[elintul]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @giladchase)

---

crates/mempool/src/mempool_test.rs line 143 at r1 (raw file):

Previously, ayeletstarkware (Ayelet Zilber) wrote…

• test_get_txs - checks order by the tip.
• test_get_txs_multi_nonce - only one account, so doesn't check order by tip
• test_get_txs_with_holes_multiple_accounts - two txs of two accounts, one with a hole, so only one tx is returned. no order check
• test_get_txs_with_holes_single_account - single account, one tx, with a hole, nothing is returned, no order check.
• test_flow_filling_holes - flow of add_tx and get_txs.

If we always check order when >= 2 transactions are returned, this test name might be confusing.

[ayeletstarkware]

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @elintul and @giladchase)

---

crates/mempool/src/mempool_test.rs line 143 at r1 (raw file):

Previously, elintul (Elin) wrote…

If we always check order when >= 2 transactions are returned, this test name might be confusing.

wdym? we don't check 2+ txs in all the tests

[elintul]

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware and @giladchase)

---

crates/mempool/src/mempool_test.rs line 147 at r2 (raw file):

#[case::test_get_more_than_all_eligible_txs(5)]
#[case::test_get_less_than_all_eligible_txs(2)]
fn test_get_txs_fetching_order(#[case] requested_txs: usize) {

Suggestion:

test_get_txs_returns_by_priority_order

[elintul]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @giladchase)