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(a3p): extend coverage of KREAd app on z:acceptance test phase #10212

Closed

Conversation

Jorge-Lopes
Copy link
Collaborator

@Jorge-Lopes Jorge-Lopes commented Oct 3, 2024

closes: https://github.com/Agoric/BytePitchPartnerEng/issues/13
refs: https://github.com/Agoric/BytePitchPartnerEng/issues/9
refs: #10049

Description

The existing test of the KREAd application on the z:acceptance test phase was limited to the feature allowing users to mint a new KREAd Character.

This PR extends the existing test coverage to include the following cases:

  • minting a new Character
  • unequip all defaults Items
  • sell unequipped Item
  • buy an Item on marketplace
  • sell a Character
  • buy a Character on marketplace

Along with the kread.test.js file, this PR includes a new file, /test-lib/kread.js, that provides all the required helper functions for the test cases above, allowing a cleaner and scalable design for KREAd tests.

Since the test coverage of the new file overlaps with the existing one, I opted for removing both:

  • create-kread-item-test.sh
  • generate-kread-item-request.mjs

Security Considerations

No security considerations were identified.

Scaling Considerations

No scaling considerations were identified.

Documentation Considerations

No documentation considerations were identified.

Testing Considerations

I have confirmed that this works fine with existing a3p tests locally, there is still an improvement that may be useful to ensure that the user wallet's purse balance for the desired KREAd asset was updated accordingly.

For this purpose, we intend to use one feature included in the currently open PR #10171 , which is the sync-tools method retryUntilCondition().

However, we hope to address those changes in a different PR that will be the result of the effort put into the currently open ticket

@Jorge-Lopes Jorge-Lopes requested a review from a team as a code owner October 3, 2024 21:17
@Jorge-Lopes Jorge-Lopes marked this pull request as draft October 8, 2024 16:42
@Jorge-Lopes
Copy link
Collaborator Author

Since this PR hasn't undergone the review process yet, I’ve decided to close it for the following reasons:

  • The PR was opened from a forked repository, while the agreed-upon procedure is to create new branches directly from the agoric-sdk repo. This will help align with the project’s workflow.
  • The commit history has become cluttered due to multiple back-and-forth changes, especially around the use of the smallCapsContext method from @agoric/synthetic-chain. Starting fresh will allow me to provide a much cleaner and organized history.

I’ll be submitting a new PR shortly, following the correct workflow and ensuring a clean commit history.

@Jorge-Lopes Jorge-Lopes closed this Oct 8, 2024
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.

1 participant