Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test baggage #9659

Merged
merged 4 commits into from
Jul 8, 2024
Merged

test baggage #9659

merged 4 commits into from
Jul 8, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Jul 6, 2024

incidental

Description

Adds a way to test contract baggage, to detect changes.

Security Considerations

Uses the setTestJig to expose baggage. Safe by design.

Scaling Considerations

none

Documentation Considerations

Could be useful at some point

Testing Considerations

CI

Upgrade Considerations

none

@turadg turadg requested a review from Chris-Hibbert July 6, 2024 19:44
Copy link

cloudflare-workers-and-pages bot commented Jul 6, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: fbe4414
Status: ✅  Deploy successful!
Preview URL: https://22a2feab.agoric-sdk.pages.dev
Branch Preview URL: https://ta-baggage-tests.agoric-sdk.pages.dev

View logs

@turadg turadg force-pushed the ta/baggage-tests branch from 08c1288 to 567db6a Compare July 7, 2024 15:11
@turadg turadg mentioned this pull request Jul 7, 2024
mergify bot added a commit that referenced this pull request Jul 8, 2024
_incidental_

## Description
Some cleanups noticed in the course of [#9659](#9659)

### Security Considerations
none
### Scaling Considerations
none

### Documentation Considerations
none
### Testing Considerations
"Dependency Graph" test has been failing on the recent change to `replay-membrane.js`. Maybe that should be a Required test in the repo.

### Upgrade Considerations
This changes Zoe but in a fully backwards and forward compatible way. I think it can get into master and reach chain whenever Zoe is next upgraded.
@turadg turadg force-pushed the ta/baggage-tests branch from 567db6a to fc34640 Compare July 8, 2024 15:35
@turadg turadg removed the request for review from Chris-Hibbert July 8, 2024 15:36
@turadg turadg force-pushed the ta/baggage-tests branch from fc34640 to 066093c Compare July 8, 2024 15:53
@turadg turadg requested review from dckc and 0xpatrickdev July 8, 2024 15:53
@turadg turadg marked this pull request as ready for review July 8, 2024 15:54
Copy link
Member

@0xpatrickdev 0xpatrickdev left a comment

Choose a reason for hiding this comment

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

LGTM, although I'm curious what behavior we are testing or what invariants we are trying to uphold.

How should we be evaluating the snapshots? Two things coming to mind -

  • userspace keys do not interfere with service level endowments
  • baggage contents are available after upgrade

@turadg
Copy link
Member Author

turadg commented Jul 8, 2024

LGTM, although I'm curious what behavior we are testing or what invariants we are trying to uphold.

Good questions. In general, I think snapshot tests are valuable for:

  • detecting changes
  • illustrating the contents

Both are helpful right now for the PRs refactoring zones for retriable and a new boilerplate abstraction.

  • userspace keys do not interfere with service level endowments

Yes, though I expect that will be maintained by a sub-baggage.

  • baggage contents are available after upgrade

These don't test upgrade per se. I take it you mean that any change observed is backwards compatible.

@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Jul 8, 2024
@turadg turadg force-pushed the ta/baggage-tests branch from 066093c to 8b014a0 Compare July 8, 2024 17:10
@turadg turadg force-pushed the ta/baggage-tests branch from 8b014a0 to fbe4414 Compare July 8, 2024 20:51
@mergify mergify bot merged commit a4a571e into master Jul 8, 2024
78 checks passed
@mergify mergify bot deleted the ta/baggage-tests branch July 8, 2024 21:23
@erights erights mentioned this pull request Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants