New test suite + improve type safety

[igorls]

No description provided.

[gemini-code-assist]

Hello @igorls, 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!

Summary of Changes

Hello team,

Gemini here with a summary of this pull request by @igorls.

This PR introduces a new, more comprehensive test suite and significantly improves the type safety and error handling within the node-abieos library. The core Abieos class has been refactored to better manage loaded ABIs internally and provide clearer error messages when interacting with the native module. Additionally, new documentation has been added to cover usage, contribution guidelines, and the build process.

Highlights

• Improved Type Safety and Error Handling: The TypeScript definition files and implementation have been updated to use BigInt for name types and add detailed JSDoc comments. Error handling has been enhanced across most Abieos methods, wrapping native calls in try...catch blocks and throwing more informative JavaScript errors with a consistent log tag.
• Abieos Class Refactoring: The Abieos singleton class now internally tracks loaded contracts using a Map. New methods getLoadedAbis() and cleanup() are added to manage the state of loaded ABIs. The loadAbi and loadAbiHex methods now correctly handle updating an already loaded ABI for a contract.
• Enhanced Documentation: New README.md and CONTRIBUTING.md files have been added to the docs directory, providing detailed user documentation, API reference, error handling, debugging tips, examples, and contribution guidelines.
• Build Process Updates: Build scripts in package.json have been updated, including a new script (scripts/copy-module.mjs) to handle copying the native module after compilation. Dependencies like axios and cross-spawn have been updated.

Changelog

Click here to see the changelog

• README.md
  • Updated description of improvements (internal loaded contract map, deleteContract).
  • Added links to new documentation files.
• dist/_tsup-dts-rollup.d.cts
  • Added JSDoc comments for Abieos class and methods.
  • Added logTag, loadedContracts, debug static properties.
  • Added getLoadedAbis() and cleanup() method declarations.
  • Changed stringToName return type from BigInteger to BigInt.
• dist/_tsup-dts-rollup.d.ts
  • Added JSDoc comments for Abieos class and methods.
  • Added logTag, loadedContracts, debug static properties.
  • Added getLoadedAbis() and cleanup() method declarations.
  • Changed stringToName return type from BigInteger to BigInt.
• dist/abieos.cjs
  • Added logTag, loadedContracts (Map), and debug static properties.
  • Improved error message in constructor.
  • Implemented getLoadedAbis() and cleanup() methods.
  • Added try...catch blocks and improved error messages in stringToName, jsonToHex, hexToJson, binToJson, loadAbi, loadAbiHex, getTypeForAction, getTypeForTable, deleteContract.
  • Updated loadAbi and loadAbiHex to track loaded contracts and log debug info.
  • Updated deleteContract to remove from tracking map and log debug info.
• dist/abieos.js
  • Added logTag, loadedContracts (Map), and debug static properties.
  • Improved error message in constructor.
  • Implemented getLoadedAbis() and cleanup() methods.
  • Added try...catch blocks and improved error messages in stringToName, jsonToHex, hexToJson, binToJson, loadAbi, loadAbiHex, getTypeForAction, getTypeForTable, deleteContract.
  • Updated loadAbi and loadAbiHex to track loaded contracts and log debug info.
  • Updated deleteContract to remove from tracking map and log debug info.
• docs/CONTRIBUTING.md
  • Added new file with contribution guidelines.
• docs/README.md
  • Added new file with detailed user documentation, API reference, error handling, debugging, examples, and build process.
• examples/serialization.mjs
  • Added new example file demonstrating serialization/deserialization using fetch.
• lib/abieos.ts
  • Added logTag, loadedContracts (Map), and debug static properties.
  • Improved error message in constructor (line 23).
  • Implemented getLoadedAbis() and cleanup() methods (lines 40-62).
  • Added try...catch blocks and improved error messages in stringToName (line 71), jsonToHex (line 88), hexToJson (line 105), binToJson (line 133), loadAbi (line 166), loadAbiHex (line 200), getTypeForAction (line 221), getTypeForTable (line 236), deleteContract (line 250).
  • Updated loadAbi and loadAbiHex to track loaded contracts and log debug info (lines 168-172, 201-205).
  • Updated deleteContract to remove from tracking map and log debug info (lines 253-255).
• package-lock.json
  • Updated axios dependency from 1.7.7 to 1.9.0.
  • Updated cross-spawn dependency from 7.0.3 to 7.0.6.
• package.json
  • Removed test:examples script.
  • Updated test:watch script to include coverage.
  • Simplified build:linux script and removed build:linux:ci.
  • Added build:mac script.
  • Updated binary.napi_versions format.
• scripts/copy-module.mjs
  • Added new script to copy the native module after build.
• test/abi_management.test.js
  • Updated imports to use new file structure.
  • Removed setupAbieos import and usage, using Abieos.getInstance() and cleanup() instead.
  • Set Abieos.debug = true.
  • Added tests for getLoadedAbis, cleanup (including error handling), loadAbi (updating existing), loadAbiHex (invalid input, saving to map), getTypeForAction, getTypeForTable, deleteContract (success and invalid input).
  • Removed clearLoadedAbis test.
  • Updated expected error messages in assertThrows calls.
• test/basic_types.test.js
  • Updated imports to use new file structure.
  • Removed setupAbieos import and usage, using Abieos.getInstance() and cleanup() instead.
  • Set Abieos.debug = true.
  • Updated expected error messages in assertThrows calls.
• test/complex_types.test.js
  • Updated imports to use new file structure.
  • Removed setupAbieos import and usage, using Abieos.getInstance() and cleanup() instead.
  • Set Abieos.debug = true.
  • Updated expected error messages in assertThrows calls.
• test/data_types.test.js
  • Updated imports to use new file structure.
  • Removed setupAbieos import and usage, using Abieos.getInstance() and cleanup() instead.
  • Set Abieos.debug = true.
• test/deserialization.test.js
  • Updated imports to use new file structure.
  • Removed setupAbieos import and usage, using Abieos.getInstance() and cleanup() instead.
  • Set Abieos.debug = true.
  • Removed hexTojson alias, using hexToJson directly.
  • Added tests for binToJson error handling.
  • Updated expected error messages in assertThrows calls.
• test/eosio_specific_types.test.js
  • Updated imports to use new file structure.
  • Removed setupAbieos import and usage, using Abieos.getInstance() and cleanup() instead.
  • Set Abieos.debug = true.
  • Added test for stringToName with invalid input.
  • Removed old stringToName and nameToString tests.
  • Updated expected error messages in assertThrows calls.
• test/serialization.test.js
  • Updated imports to use new file structure.
  • Removed setupAbieos import and usage, using Abieos.getInstance() and cleanup() instead.
  • Set Abieos.debug = true.
  • Updated expected error messages in assertThrows calls.
• test/singleton.test.js
  • Added new test file for the Abieos singleton pattern.
• test/test-helpers.js
  • Deleted file.
• test/type_conversion.test.js
  • Deleted file (tests moved to abi_management.test.js).
• test/utils/eosio.token.raw
  • Added new file containing a base64 encoded raw ABI.
• test/utils/test-helpers.js
  • Added new file with updated assertThrows and testRoundTrip helpers.
• tsconfig.json
  • Added outDir: "dist" option.

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 is currently in preview and 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 to provide feedback.

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.

---

New tests arrive,
Types now feel alive,
Errors caught with care,
Code quality to share,
A better binding thrives.

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 introduces a significant set of improvements, including a new test suite, enhanced type safety, much-improved error handling in the Abieos class wrapper, and comprehensive documentation. These are excellent additions that will greatly benefit the project's robustness and usability.

The core changes to the Abieos class, such as the introduction of logTag, debug mode, loadedContracts tracking, and the cleanup method, are well-implemented. The error messages are now more descriptive and consistently prefixed, which will aid in debugging. The transition from a custom BigInteger to the standard BigInt is a good step for type safety. The new documentation in the docs/ directory is also a very valuable contribution.

However, there's a critical issue in the test helper assertThrows that needs to be addressed, as it impacts the reliability of the new test suite.

Summary of Findings

• Test Helper assertThrows Logic: The assertThrows helper function in test/utils/test-helpers.js has a critical flaw where it prematurely returns if an error message contains Abieos.logTag, bypassing the specific error message check. This can lead to tests passing even if the wrong error is thrown.
• Accuracy of Error Message Assertions in Tests: Due to the issue in assertThrows, many tests that assert specific error messages (often changed to generic strings like 'failed to parse data') might not be correctly verifying the error messages. These assertions need to be reviewed and updated after assertThrows is fixed.
• Behavioral Change in loadAbi: The loadAbi and loadAbiHex methods now allow updating an existing ABI by reloading it. This is a change from previous behavior and should be clearly documented for users.
• Documentation and Test Suite Expansion: The PR includes excellent new documentation and a significantly expanded and improved test suite. These are major enhancements.
• Improved Error Handling and Type Safety: The Abieos class wrapper has much more robust error handling and improved type safety (e.g., use of BigInt).

Merge Readiness

This pull request contains many valuable improvements. However, the identified critical issue with the assertThrows test helper and the related need to verify error message assertions in the tests should be addressed before merging. Once these are resolved, the PR will be in a much stronger position. I am not authorized to approve pull requests, so please ensure further review and approval from authorized maintainers after addressing the feedback. The behavioral change in loadAbi should also be considered for documentation.

[gemini-code-assist]

The current logic in assertThrows might lead to tests passing incorrectly. If an error message includes Abieos.logTag (which all errors thrown by the Abieos wrapper methods do), the function returns on line 18, bypassing the subsequent checks against expectedError (lines 21-27).

This means if a test specifies an expectedError (string or RegExp), but the actual error thrown by the Abieos wrapper (containing the logTag) is different, the test will still pass as long as an error with the logTag was thrown. This undermines the specificity of error message testing.

Could we refactor this to ensure that if expectedError is provided, it's always checked, regardless of the presence of Abieos.logTag? The logTag check could perhaps be a fallback or an additional assertion if no specific expectedError is given?

[igorls]

refactor done

[igorls]

/gemini review

[Copilot]

Pull Request Overview

This PR removes legacy test helper files and adapts tests to directly use the Abieos singleton, while updating error message expectations and improving type safety. Key changes include:

• Removal of test/type_conversion.test.js and test/test-helpers.js in favor of utils/test-helpers.js and direct instance usage.
• Updates across test suites for consistent method naming (e.g. hexToJson) and improved error regex expectations.
• Enhancements to lib/abieos.ts with added logging, contract tracking via a static map, and a new cleanup method.

Reviewed Changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.

File	Description
test/*	Updates to test files using Abieos.getInstance() instead of setupAbieos, along with updated error regex patterns.
lib/abieos.ts	Refactored error messages, added a static loadedContracts map, and introduced a cleanup method with enhanced logging.
scripts/*, package.json, docs	Version bump, new build scripts, and expanded documentation.

[Copilot]

The test 'cleanup should throw if an invalid contract name is found...' expects an exception when an invalid contract name is encountered, but the cleanup() method currently only logs the error without rethrowing it. Consider updating cleanup() to rethrow errors (or adjust the test expectation) so that erroneous contract names are properly surfaced.

[Copilot]

Currently, the cleanup() method catches errors from delete_contract() and only logs them. For clearer feedback and to support robust testing, consider rethrowing the error or providing a mechanism to signal cleanup failures rather than silently logging them.

Suggested change

	console.error(`${Abieos.logTag} Failed to delete contract '${contractName}' during cleanup: ${e.message}`);
	}
	});
	const errorMessage = `${Abieos.logTag} Failed to delete contract '${contractName}' during cleanup: ${e.message}`;
	console.error(errorMessage);
	errors.push(errorMessage);
	}
	});
	if (errors.length > 0) {
	throw new Error(`${Abieos.logTag} Cleanup encountered errors:\n` + errors.join('\n'));
	}

[gemini-code-assist]

Code Review

This pull request introduces a new test suite, significantly improves type safety by migrating to BigInt, enhances error handling with more descriptive messages, and adds better management for loaded ABIs including a cleanup utility and debug logging. The addition of comprehensive documentation is also a major plus. Overall, these are excellent improvements to the library's robustness and usability.

Summary of Findings

• Error Handling in cleanup Method: The cleanup method currently logs errors during contract deletion to console.error but does not propagate these errors to the caller. This might hide issues in automated environments. Suggesting to either throw an aggregate error or return information about failed deletions.
• Test Suite and Type Safety: The PR introduces a significantly improved test suite and enhances type safety, notably by using BigInt. These are excellent changes.
• Documentation: The addition of docs/README.md and docs/CONTRIBUTING.md greatly improves the project's documentation.
• .npmignore: Consider adding node_modules to the .npmignore file to prevent it from being included in the published package. I have added a review comment for this.

Merge Readiness

The pull request is in good shape with significant improvements. Addressing the suggested change to error handling in the cleanup method and adding node_modules to .npmignore would further enhance its robustness. After considering these points, the PR should be ready for merge. As an AI, I am not authorized to approve pull requests; please ensure further review and approval from the maintainers.

[gemini-code-assist]

The cleanup method currently catches errors that occur when deleting a contract and logs them to console.error, but it doesn't re-throw them or otherwise signal to the caller that some cleanup operations may have failed. For a library function, especially one that might be used in automated tests or shutdown procedures, it might be beneficial to provide a way for the caller to be aware of these failures.

Could we consider an alternative approach, such as collecting all errors encountered during cleanup and then throwing an aggregate error at the end, or returning an array of errors/failed contract names? This would give the caller more control over handling such situations.

   */
  public cleanup(): void {
    const errors: any[] = [];
    Abieos.loadedContracts.forEach((_, contractName) => {
      try {
        if (Abieos.debug) {
          console.log(`${Abieos.logTag} Cleaning up contract '${contractName}'...`);
        }
        Abieos.native.delete_contract(contractName);
        Abieos.loadedContracts.delete(contractName);
      } catch (e: any) {
        errors.push({contractName, error: e.message});
        console.error(`${Abieos.logTag} Failed to delete contract '${contractName}' during cleanup: ${e.message}`);
      }
    });

    if (errors.length > 0) {
      throw new Error(`${Abieos.logTag} Errors during cleanup: ${JSON.stringify(errors)}`);
    }
  }