test: tools for sync operations #10171
Merged
+619
−7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @LuqiPan |
LuqiPan
requested review from
Chris-Hibbert
and removed request for
AgoricTriage
Chris-Hibbert
requested changes
Oct 3, 2024
a3p-integration/proposals/z:acceptance/test-lib/sync-tools.test.js
Outdated
Show resolved
Hide resolved
a3p-integration/proposals/z:acceptance/test-lib/sync-tools.test.js
Outdated
Show resolved
Hide resolved
a3p-integration/proposals/z:acceptance/test-lib/sync-tools.test.js
Outdated
Show resolved
Hide resolved
This was referenced Oct 5, 2024
anilhelvaci
force-pushed
the
anil/sync-tools
branch
2 times, most recently
from
57e25e6
to
0b1ace6
Compare
|
Thanks a lot for your comprehensive review. Implemented the changes you requested. @Chris-Hibbert |
There was a problem hiding this comment.
I just discovered that the lint checking we have set up in our other repos isn't being applied here. Rather than pointing out individual deviations, I'm going to request that our SRE team add enforcement here.
Once that's in place, I'll ask for a clean-up pass, but it doesn't make sense to request thorough compliance until the tooling is in place.
I'll come back to this review once that has been configured.
|
Yep, I'm aware of that. But I thought this was something known. As In this case, other |
Not as I understand the process. I've filed a ticket, which should be fielded by our SRE team. They often are able to address simple issues like this overnight. |
anilhelvaci
added a commit
to bytepitch/agoric-sdk
that referenced
this pull request
Oct 8, 2024
* change `performAction.js` name to `submitBid.js` * remove `Math.round` from `scale6` * update indexing of bids and other constants in `config` object to improve readability (`auction.test.js`) * move helper functions in `auction.test.js` to `test-lib/auction-lib.js` * move `lib/vaults.mts` to `test-lib/vaults.mts` and remove empty `lib` directory * let it be known `sync-tools.js` is a stand-in code for Agoric#10171 Refs: Agoric/BytePitchPartnerEng#8
anilhelvaci
added a commit
to bytepitch/agoric-sdk
that referenced
this pull request
Oct 8, 2024
* change `performAction.js` name to `submitBid.js` * remove `Math.round` from `scale6` * update indexing of bids and other constants in `config` object to improve readability (`auction.test.js`) * move helper functions in `auction.test.js` to `test-lib/auction-lib.js` * move `lib/vaults.mts` to `test-lib/vaults.mts` and remove empty `lib` directory * let it be known `sync-tools.js` is a stand-in code for Agoric#10171 Refs: Agoric/BytePitchPartnerEng#8
anilhelvaci
added a commit
that referenced
this pull request
Oct 8, 2024
…style fixes * change `performAction.js` name to `submitBid.js` * remove `Math.round` from `scale6` * update indexing of bids and other constants in `config` object to improve readability (`auction.test.js`) * move helper functions in `auction.test.js` to `test-lib/auction-lib.js` * move `lib/vaults.mts` to `test-lib/vaults.mts` and remove empty `lib` directory * let it be known `sync-tools.js` is a stand-in code for #10171 Refs: Agoric/BytePitchPartnerEng#8 fix: style fixes
anilhelvaci
force-pushed
the
anil/sync-tools
branch
from
0b1ace6
to
3db629b
Compare
Merged
mergify bot
added a commit
that referenced
this pull request
Oct 15, 2024
closes: #10238 ## Description Lint a3p-integration for consistency and stopping red squiggles in IDEs Should unblock #10171 once it rebases on this ### Security Considerations n/a ### Scaling Considerations n/a ### Documentation Considerations The config should explain ### Testing Considerations per se ### Upgrade Considerations none
anilhelvaci
force-pushed
the
anil/sync-tools
branch
2 times, most recently
from
fd37b7c
to
3da0fa5
Compare
anilhelvaci
changed the title
Chris-Hibbert
approved these changes
Oct 18, 2024
|
No worries @Chris-Hibbert. Should I initiate the merge then? |
Refs: Agoric/BytePitchPartnerEng#10 fix: formatting fixes and change requests fix(sync-tools): apply new linting rules and fix rebase conflicts Refs: Agoric#10238
anilhelvaci
force-pushed
the
anil/sync-tools
branch
from
3da0fa5
to
dfd6be1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refs: Agoric/BytePitchPartnerEng#10
refs: Agoric/agoric-3-proposals#181
Description
Currently the way to make sure a transaction sent to agoric chain has been executed is to use a method from
@agoric/synthetic-chaincalledwaitForBlockwhich stops the execution flow of a given ava test until N number of blocks are produced. This is not as deterministic as we'd like it to be but so far it worked fine. However, it could still end up in a race condition in some extreme cases. In this PR we introduce a set of tools to sync operations carried out in a test. Currently this PR supports operations like;See Agoric/agoric-3-proposals#181 for further discussion.
Security Considerations
This is merely a testing tool, so no security considerations.
Scaling Considerations
This is merely a testing tool, so no scaling considerations.
Documentation Considerations
If this ends up getting in to
@agoric/synthetic-chainthen we might need more extensive explanation of what we do. But for now, I don't think any documentation is needed.Testing Considerations
Have confirmed that this works fine with existing a3p tests locally but to keep the scope tighter I will update those tests one this PR lands. The coverage of the unit tests in
sync-tools.test.jsseems fine but I'm open to suggestions, of course.