Skip to content

Conversation

@nullun
Copy link
Contributor

@nullun nullun commented Aug 22, 2025

When working on #6396 I added a simple test, but this PR should cover every asset types wellFormed errors.

I also added three simple 0 asset e2e tests, to make it easier to notice if anything there changes.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @nullun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the test suite by introducing comprehensive "wellFormed" error checks for various asset transaction types and adding new end-to-end tests to cover edge cases related to zero asset transfers.

Highlights

  • Asset Transaction Error Testing: New "TestAxferWellFormedErrors", "TestAcfgWellFormedErrors", and "TestAfrzWellFormedErrors" functions were added in "data/transactions/asset_test.go" to thoroughly test "wellFormed" error conditions for asset transfer, configuration, and freeze transactions, ensuring robust validation.
  • Zero Asset Transfer E2E Tests: Three new end-to-end test cases were introduced in "test/scripts/e2e_subs/asset-misc.sh" to specifically validate the behavior of transferring zero units of asset 0, covering scenarios like transfers to self, to another account, and transfers with a close-to address.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds valuable test coverage for wellFormed errors in asset transactions and includes new end-to-end tests for asset 0 transfers. The changes are well-aligned with the goal of improving test coverage and ensuring protocol stability. My feedback focuses on enhancing the structure of the new Go tests to improve their readability and maintainability by following common Go testing patterns.

@gmalouf gmalouf requested a review from jannotti September 24, 2025 15:02
Copy link
Contributor

@jannotti jannotti left a comment

Choose a reason for hiding this comment

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

I'm ok with after the scriptname typo is fixed.

@nullun nullun force-pushed the tests-for-asset-transactions branch from b961b0d to 049519a Compare September 25, 2025 09:51
@codecov
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.74%. Comparing base (fc6cac2) to head (049519a).
⚠️ Report is 27 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6417      +/-   ##
==========================================
- Coverage   50.82%   50.74%   -0.08%     
==========================================
  Files         664      658       -6     
  Lines      111373   111446      +73     
==========================================
- Hits        56606    56557      -49     
- Misses      51893    52010     +117     
- Partials     2874     2879       +5     

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

@cce cce requested a review from Copilot October 2, 2025 15:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances test coverage for asset-related functionality by adding comprehensive validation tests for asset transaction field errors and extending e2e tests for zero asset transfers.

  • Adds comprehensive unit tests for wellFormed error validation across all asset transaction types (axfer, acfg, afrz)
  • Extends e2e tests with three scenarios for zero asset transfers to catch behavioral changes
  • Fixes a minor logging inconsistency in existing asset-misc.sh test

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/scripts/e2e_subs/asset-misc.sh Fixes logging format consistency and adds three zero asset transfer test scenarios
data/transactions/asset_test.go New comprehensive test file covering wellFormed validation errors for all asset transaction types

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants