chore(ERTP): AmountMath compare should be same as Key compare #6769
Conversation
| ) | ||
| .filter(pairs => distinctLabels(pairs) && positiveCounts(pairs)); | ||
|
|
||
| // TODO: should include many non-bag amounts too |
erights
force-pushed
the
markm-test-amount-key-arith
branch
from
c19079a
to
4e96088
Compare
|
@kriskowal I suspect the CI failure at https://github.com/Agoric/agoric-sdk/actions/runs/3880814040/jobs/6619258891 is really a bundler problem. But I have no idea how to proceed and need some help. |
erights
changed the title
erights
force-pushed
the
markm-test-amount-key-arith
branch
from
4e96088
to
3f0b3b2
Compare
erights
force-pushed
the
markm-test-amount-key-arith
branch
from
3f0b3b2
to
307a994
Compare
erights
force-pushed
the
markm-test-amount-key-arith
branch
2 times, most recently
from
b8b3861
to
f394f66
Compare
| test('amount equality iff key equality', async t => { | ||
| await fc.assert( | ||
| fc.property(fc.record({ x: arbAmount, y: arbAmount }), ({ x, y }) => { | ||
| return t.true(m.isEqual(x, y) === keyEQ(x, y)); | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| test('amount >= iff key >=', async t => { | ||
| await fc.assert( | ||
| fc.property(fc.record({ x: arbAmount, y: arbAmount }), ({ x, y }) => { | ||
| return t.true(m.isGTE(x, y) === keyGTE(x, y)); | ||
| }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Actually, the premise of this PR is wrong, and these tests need to be revised to reflect that.
We currently have four kinds of Amounts:
- nats
- sets represented as arrays -- deprecated but widely used
- copySets, which should be used instead of arrays representing sets
- copyBags, for semi-fungibles
The premise of this PR is true for nats, copySets, and copyBags. But it is not true for deprecated arrays representing sets, so we need to exclude them from these tests.
(Separately, we will be deprecating copySets as well, switching all uses of arrays-representing-sets and copySets to copyBags. But this is not a concern of this PR.)
erights
force-pushed
the
markm-test-amount-key-arith
branch
2 times, most recently
from
921552a
to
c6ca4d0
Compare
Datadog ReportBranch report: ✅ |
erights
force-pushed
the
markm-test-amount-key-arith
branch
from
c6ca4d0
to
555c6dc
Compare
Datadog ReportBranch report: ✅ |
erights
force-pushed
the
markm-test-amount-key-arith
branch
from
555c6dc
to
7f75877
Compare
erights
force-pushed
the
markm-test-amount-key-arith
branch
from
7f75877
to
5550551
Compare
|
The invariant that this PR tests is a possible reason to prefer tight pareto to loose pareto. Or maybe not. We need to think about this. |
Tests that AmountMath equality and inequality match Key equality and inequality.
Moves the arbPassable abstraction into its own tools/ file so it can be shared by tests, including potentially tests outside this package. See precedent to be set by endojs/endo#1291
While explaining the store level of abstraction to @Tyrosine22 , I mentioned that we defined the key compare on amounts to be aligned with amount compare. Then I realized that this would be a perfect subject for property-based testing. However, the current arb-amount.js is too narrow for this test to really mean anything yet. It will need to generate many more kinds of amounts.