feat: cucumber ci

[SWvheerden]

Description

Adds cucumber tests to ci
fixes a few bugs

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 project's testing infrastructure by integrating Cucumber tests into the continuous integration pipeline. It also includes several bug fixes and improvements across the API, database interactions, and transaction logic, making the system more robust and flexible, particularly for handling multi-recipient transactions and providing clearer API responses.

Highlights

• CI/CD Integration: Integrated Cucumber tests into the continuous integration pipeline, enhancing automated testing capabilities.
• Bug Fixes and Enhancements: Addressed several bugs, including improved string masking, refined balance checks for transaction input selection, and corrected database queries for output fetching. The API's scan_status endpoint now provides default values instead of a 404 when no blocks have been scanned.
• Transaction Flexibility: Removed the previous restriction that limited one-sided transactions to a single recipient, now allowing for multiple recipients.

Changelog

• .cargo/config.toml
  • Updated the ci-cucumber command definition.
• integration-tests/features/fund_locking.feature
  • Modified a Then step to reflect UTXO locking status more accurately.
  • Added a @pie tag to a scenario.
• integration-tests/features/mining.feature
  • Removed the "Sync between two nodes" scenario.
• integration-tests/features/transactions.feature
  • Removed the "the transaction file should be created" step.
  • Removed the "Create transaction with custom lock duration" scenario.
  • Added a blank line for formatting.
• integration-tests/steps/common.rs
  • Changed the database_with_wallet function to be public.
• integration-tests/steps/daemon.rs
  • Added a generate_test_address helper function.
  • Ensured setup_database is called when starting a daemon process.
  • Ensured a wallet is imported before starting a daemon with an existing wallet.
  • Updated the lock_funds API request to use amount instead of amount_microtari.
  • Refactored the create_unsigned_transaction API request to use a generated address, updated amount, removed fee_per_gram, and added payment_id.
  • Implemented storing API transaction responses for subsequent step assertions.
  • Updated assertions for balance information in API responses.
  • Updated assertions for unsigned transaction API responses to check tx_id or info fields.
• integration-tests/steps/fund_locking.rs
  • Stored the output file path in world.output_file.
  • Removed the --password argument from the lock-funds command.
  • Added logic to parse and store locked funds JSON output if the command succeeds.
• integration-tests/steps/transactions.rs
  • Stored the output file path in world.output_file.
  • Added logic to parse and store transaction JSON output if the command succeeds.
  • Removed the transaction_file_created verification step.
  • Adjusted transaction verification steps to access data from the info object within the stored transaction JSON.
  • Removed a blank line in wallet_has_balance and wallet_has_zero_balance.
• minotari/src/api/accounts.rs
  • Updated documentation and error handling for the scan_status endpoint to return default values instead of a 404 when no blocks are scanned.
  • Removed redundant blank lines in api_lock_funds and api_create_unsigned_transaction.
• minotari/src/db/outputs.rs
  • Added Debug derive to WalletOutputRow.
  • Changed the query for unspent outputs to use mined_in_block_height instead of confirmed_height.
• minotari/src/log/mod.rs
  • Improved the mask_string function to correctly handle UTF-8 characters when masking.
• minotari/src/main.rs
  • Removed a blank line in handle_create_unsigned_transaction.
• minotari/src/transactions/fund_locker.rs
  • Removed a blank line in the lock function.
• minotari/src/transactions/input_selector.rs
  • Removed blank lines.
  • Adjusted the balance check logic to ensure total_unspent_balance >= amount when checking available_balance <= amount.
• minotari/src/transactions/one_sided_transaction.rs
  • Removed the restriction that only one recipient is supported for one-sided transactions.
  • Updated logging to show recipient count instead of individual recipient details.
  • Refactored payment_id handling to support multiple recipients and use the first recipient's payment_id as the overall transaction memo.

Ignored Files

• Ignored by pattern: .github/workflows/** (1)
  • .github/workflows/ci.yml

Activity

• The pull request was created by SWvheerden.
• The pull request aims to add Cucumber tests to CI and fix several bugs.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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. ↩

[gemini-code-assist]

Code Review

This pull request adds Cucumber tests to the CI pipeline and includes several bug fixes and refactorings. My review focuses on a critical typo in the CI configuration that would cause it to fail, the removal of several test scenarios which reduces test coverage, and a few opportunities for code improvement, such as adding more robust error handling and removing redundant code. Overall, the changes improve the test infrastructure, but care should be taken not to lose valuable test cases in the process.

I am having trouble creating individual review comments. Click here to see my feedback.

.cargo/config.toml (7)

There appears to be a typo in the ci-cucumber command. cucumbe should likely be cucumber. This will probably cause the CI step to fail.

ci-cucumber = "test --release --test cucumber"

integration-tests/features/mining.feature (17-22)

The "Sync between two nodes" scenario has been removed. While this might be necessary to get CI passing if the test is flaky, removing tests reduces coverage. It would be better to fix the underlying issue or temporarily ignore the test with a ticket to track the fix. Please consider adding this scenario back, perhaps with an @ignore tag if it's currently failing.

integration-tests/features/transactions.feature (34-38)

The "Create transaction with custom lock duration" scenario has been removed. Similar to other test removals in this PR, this reduces test coverage. If this feature is still supported, the test should be kept. If it's flaky, consider ignoring it temporarily with a tracking issue rather than deleting it.

integration-tests/steps/daemon.rs (25)

Using .unwrap() can lead to less informative panic messages in case of an error. It's better practice to use .expect() with a descriptive message to aid in debugging.

    .expect("Failed to create dual address");

integration-tests/steps/transactions.rs (43)

world.output_file is being set here, but it was already set on line 22 with a clone of output_path. This second assignment is redundant. It's best to remove this line to avoid confusion and potential issues if output_path were to be used after this move.

minotari/src/transactions/one_sided_transaction.rs (241-242)

The use of unwrap_or_else here silently ignores potential errors when creating a MemoField from a payment ID string (e.g., if the string is too long). This could lead to a transaction being created with an empty memo without any indication to the user that their provided payment ID was invalid. It would be better to at least log a warning when new_open_from_string fails.

                    Some(s) => MemoField::new_open_from_string(s, TxType::PaymentToOther)
                        .unwrap_or_else(|e| {
                            log::warn!("Invalid payment ID provided: '{}'. Error: {}. Using empty memo.", s, e);
                            MemoField::new_empty()
                        }),

[martinserts]

--test cucumbe(r) ?