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.

Sign up for GitHub

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

Add Code Coverage Report #131

Merged
merged 4 commits into from
Jan 9, 2025
Merged

Add Code Coverage Report #131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bb954cc
Initial
dev2-nomo Jan 9, 2025
41ae960
fix: correct package reference in coverage formatting command
dev2-nomo Jan 9, 2025
f0e73dd
fix: set dynamic artifact name for coverage uploads
dev2-nomo Jan 9, 2025
c453c59
fix: update coverage badge path and improve JSON endpoint for shields…
dev2-nomo Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 66 additions & 18 deletions .github/workflows/dart.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,34 +88,73 @@ jobs:
dart pub get
(cd packages/bip39 && dart pub get)

- name: Run CI tests
- name: Run CI tests with coverage
run: |
echo "::group::Running tests for ${{ matrix.test-path }}"
dart test ${{ matrix.test-path }}
dart pub global activate coverage
dart test --coverage=coverage ${{ matrix.test-path }}
dart pub global run coverage:format_coverage --lcov --in=coverage --out=coverage/lcov.info --package=. --report-on=lib
echo "::endgroup::"

generate-docs:
- name: Set artifact name
run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> $GITHUB_ENV

- name: Upload coverage artifacts
uses: actions/upload-artifact@v3
with:
name: ${{ env.ARTIFACT_NAME }}
path: coverage/lcov.info
Comment on lines +99 to +106
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix shell script quoting.

The artifact name construction should use quotes to prevent word splitting.

-        run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> $GITHUB_ENV
+        run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> "$GITHUB_ENV"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Set artifact name
run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> $GITHUB_ENV
- name: Upload coverage artifacts
uses: actions/upload-artifact@v3
with:
name: ${{ env.ARTIFACT_NAME }}
path: coverage/lcov.info
- name: Set artifact name
run: echo "ARTIFACT_NAME=coverage-$(echo '${{ matrix.test-path }}' | tr '/' '-')" >> "$GITHUB_ENV"
- name: Upload coverage artifacts
uses: actions/upload-artifact@v3
with:
name: ${{ env.ARTIFACT_NAME }}
path: coverage/lcov.info
🧰 Tools
🪛 actionlint (1.7.4)

100-100: shellcheck reported issue in this script: SC2086:info:1:81: Double quote to prevent globbing and word splitting

(shellcheck)

103-103: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


coverage-report:
needs: test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- name: Download coverage reports
uses: actions/download-artifact@v3
with:
path: coverage-artifacts

- name: Install lcov
run: sudo apt-get install -y lcov

- name: Merge coverage reports
run: |
mkdir -p coverage
find coverage-artifacts -name lcov.info -exec echo -a {} \; | xargs lcov -o coverage/lcov.info

Comment on lines +122 to +126
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use null-terminated file handling for safety.

The current find/xargs command may break with filenames containing spaces or special characters.

-          find coverage-artifacts -name lcov.info -exec echo -a {} \; | xargs lcov -o coverage/lcov.info
+          find coverage-artifacts -name lcov.info -print0 | xargs -0 lcov -o coverage/lcov.info
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Merge coverage reports
run: |
mkdir -p coverage
find coverage-artifacts -name lcov.info -exec echo -a {} \; | xargs lcov -o coverage/lcov.info
- name: Merge coverage reports
run: |
mkdir -p coverage
find coverage-artifacts -name lcov.info -print0 | xargs -0 lcov -o coverage/lcov.info
🧰 Tools
🪛 actionlint (1.7.4)

123-123: shellcheck reported issue in this script: SC2038:warning:2:1: Use 'find .. -print0 | xargs -0 ..' or 'find .. -exec .. +' to allow non-alphanumeric filenames

(shellcheck)

- name: Generate HTML report
run: genhtml coverage/lcov.info --output-directory coverage/html

- name: Calculate coverage percentage
run: |
COVERAGE=$(lcov --summary coverage/lcov.info | grep "lines......" | cut -d ' ' -f 4 | cut -d '%' -f 1)
echo "COVERAGE=$COVERAGE" >> $GITHUB_ENV
mkdir -p coverage/badges
# Create shields.io compatible JSON endpoint
echo "{\"schemaVersion\": 1, \"label\": \"coverage\", \"message\": \"${COVERAGE}%\", \"color\": \"$(if [ $(echo "$COVERAGE >= 80" | bc -l) -eq 1 ]; then echo 'brightgreen'; elif [ $(echo "$COVERAGE >= 60" | bc -l) -eq 1 ]; then echo 'yellow'; else echo 'red'; fi)\"}" > coverage/badges/coverage.json

- name: Upload coverage report
uses: actions/upload-artifact@v3
with:
name: coverage-report
path: coverage/html

deploy-docs-and-coverage:
if: github.ref == 'refs/heads/main'
needs: [lint, test]
needs: [coverage-report, lint, test]
runs-on: ubuntu-latest
permissions:
contents: write # Required for pushing to gh-pages
pages: write # Required for deploying to Pages
id-token: write # Required for authentication
contents: write
pages: write
id-token: write

steps:
- uses: actions/checkout@v3
with:
submodules: "recursive"
- uses: actions/cache@v3
with:
path: |
~/.pub-cache
.dart_tool
packages/bip39/.dart_tool
key: ${{ runner.os }}-dart-${{ hashFiles('**/pubspec.lock') }}
restore-keys: ${{ runner.os }}-dart-


- name: Setup Dart SDK
uses: dart-lang/setup-dart@v1
with:
Expand All @@ -134,18 +173,27 @@ jobs:
- name: Generate API docs
run: dart pub global run dartdoc

- name: Download coverage report
uses: actions/download-artifact@v3
with:
name: coverage-report
path: coverage-report

- name: Prepare docs directory
run: |
rm -rf docs
mkdir -p docs
cp -R doc/api/* docs/
mkdir -p docs/coverage docs/badges
cp -R coverage-report/* docs/coverage/
cp -R coverage/badges/* docs/badges/

- name: Deploy to Github Pages
uses: peaceiris/actions-gh-pages@v3
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
publish_dir: ./docs
force_orphan: true
commit_message: "docs: update documentation"
commit_message: "docs: update documentation and coverage"
enable_jekyll: false
full_commit_message: "docs: update API documentation"
full_commit_message: "docs: update API documentation and coverage report"
Comment on lines +197 to +199
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add newline at end of file.

Add a newline character at the end of the file to follow POSIX standards.

           full_commit_message: "docs: update API documentation and coverage report"
+
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
commit_message: "docs: update documentation and coverage"
enable_jekyll: false
full_commit_message: "docs: update API documentation"
full_commit_message: "docs: update API documentation and coverage report"
commit_message: "docs: update documentation and coverage"
enable_jekyll: false
full_commit_message: "docs: update API documentation and coverage report"
🧰 Tools
🪛 yamllint (1.35.1)

[error] 199-199: no new line character at the end of file

(new-line-at-end-of-file)

86 changes: 55 additions & 31 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
[![TestSuite](https://github.com/nomo-app/walletkit-dart/actions/workflows/dart.yml/badge.svg?branch=main)](https://github.com/nomo-app/walletkit-dart/actions/workflows/dart.yml)
# WalletKit-Dart
[![Coverage](https://img.shields.io/endpoint?url=https://dev.nomo.app/walletkit-dart/badges/coverage.json)](https://dev.nomo.app/walletkit-dart/coverage/)

# WalletKit-Dart

WalletKit-Dart provides features for interacting with Bitcoin/Ethereum/Tron/ZENIQ/ZENIQ Smartchain/Polygon/Binance Smartchain/Litecoin/Bitcoin Cash/Eurocoin as well as several layer 2 chains.
If needed, it is easy to expand WalletKit-Dart with other chains.
WalletKit-Dart provides features for interacting with
Bitcoin/Ethereum/Tron/ZENIQ/ZENIQ Smartchain/Polygon/Binance
Smartchain/Litecoin/Bitcoin Cash/Eurocoin as well as several layer 2 chains. If
needed, it is easy to expand WalletKit-Dart with other chains.

See the [Uniswap WebOn](https://github.com/nomo-app/uniswap-webon) or the Unit-tests for a practical example how to use WalletKit-Dart.
See the [Uniswap WebOn](https://github.com/nomo-app/uniswap-webon) or the
Unit-tests for a practical example how to use WalletKit-Dart.

See the [api-docs](https://dev.nomo.app/walletkit-dart) for a list of individual functions.
See the [api-docs](https://dev.nomo.app/walletkit-dart) for a list of individual
functions.

## Features

Expand All @@ -20,16 +25,21 @@ See the [api-docs](https://dev.nomo.app/walletkit-dart) for a list of individual

## Why WalletKit-Dart?

WalletKit-Dart has been inspired by the [WalletKit-C](https://github.com/blockset-corp/walletkit) from blockset-corp and by [bitcoin_flutter](https://github.com/dart-bitcoin/bitcoin_flutter) from the dart-bitcoin-project.
WalletKit-Dart has been inspired by the
[WalletKit-C](https://github.com/blockset-corp/walletkit) from blockset-corp and
by [bitcoin_flutter](https://github.com/dart-bitcoin/bitcoin_flutter) from the
dart-bitcoin-project.

The WalletKit-C was one of the first WalletKits that combined UTXO-chains and EVM-chains under a unified class hierarchy.
However, the WalletKit-C was plagued by race conditions and memory corruptions.
Also, the WalletKit-C had an "object oriented architecture" that was poorly supported by the C-language.
The WalletKit-C was one of the first WalletKits that combined UTXO-chains and
EVM-chains under a unified class hierarchy. However, the WalletKit-C was plagued
by race conditions and memory corruptions. Also, the WalletKit-C had an "object
oriented architecture" that was poorly supported by the C-language.

bitcoin_flutter worked well, but bitcoin_flutter did not support modern null-safe Dart and it was difficult to expand for multiple chains.
bitcoin_flutter worked well, but bitcoin_flutter did not support modern
null-safe Dart and it was difficult to expand for multiple chains.

WalletKit-Dart has been developed to solve all those problems.
WalletKit-Dart works with modern Dart-versions and is easy to expand for multiple chains.
WalletKit-Dart has been developed to solve all those problems. WalletKit-Dart
works with modern Dart-versions and is easy to expand for multiple chains.

## How to integrate

Expand All @@ -53,14 +63,17 @@ Afterwards, clone submodules with:
git submodule update --init --recursive
```

The `--recursive` is important because WalletKit-Dart depends on other grandchild-submodules.
The `--recursive` is important because WalletKit-Dart depends on other
grandchild-submodules.

## Architecture

WalletKit-Dart is built for both _UTXO-chains_ and _EVM-chains_.
WalletKit-Dart provides a class hierarchy where both `UTXOTransaction` and `EVMTransaction` inherit from a `GenericTransaction` base class.
WalletKit-Dart is built for both _UTXO-chains_ and _EVM-chains_. WalletKit-Dart
provides a class hierarchy where both `UTXOTransaction` and `EVMTransaction`
inherit from a `GenericTransaction` base class.

Here is a quick summary if you do not yet understand the difference between UTXO and EVM:
Here is a quick summary if you do not yet understand the difference between UTXO
and EVM:

```
UTXO-based chains and EVM-based chains are two different architectures for blockchain systems.
Expand All @@ -75,32 +88,36 @@ In summary, UTXO-based chains focus on tracking ownership of tokens, while EVM-b

## API Philosophy

WalletKit-Dart provides a _stateless API_.
Neither does it store seed phrases, nor does it store transactions.
It is the users responsibility to store any needed data.
WalletKit-Dart provides a _stateless API_. Neither does it store seed phrases,
nor does it store transactions. It is the users responsibility to store any
needed data.

By design, WalletKit-Dart does not have any persistent databases.
Instead, WalletKit-Dart only has a few in-memory-caches to improve performance of repeated calls.
This design helps to simplify the API.
By design, WalletKit-Dart does not have any persistent databases. Instead,
WalletKit-Dart only has a few in-memory-caches to improve performance of
repeated calls. This design helps to simplify the API.

Moreover, WalletKit-Dart provides different APIs for UTXO-chains and EVM-chains.
Although every transaction inherits from a generic base transaction, we want to provide APIs that are specifically targeted for the architecture of a chain.
In that sense, we deviate from a traditional object oriented approach.
Although every transaction inherits from a generic base transaction, we want to
provide APIs that are specifically targeted for the architecture of a chain. In
that sense, we deviate from a traditional object oriented approach.

In other words, while we aim to reuse code between chains, we do not want to create broken abstraction layers by abstracting too much complexity away.
In other words, while we aim to reuse code between chains, we do not want to
create broken abstraction layers by abstracting too much complexity away.

## Backend APIs

Depending on the chain, WalletKit-Dart depends on multiple backend APIs.
For UTXO-chains, WalletKit-Dart depends on the [ElectrumX Protocol](https://electrumx.readthedocs.io/en/latest/protocol-methods.html).
Depending on the chain, WalletKit-Dart depends on multiple backend APIs. For
UTXO-chains, WalletKit-Dart depends on the
[ElectrumX Protocol](https://electrumx.readthedocs.io/en/latest/protocol-methods.html).

For EVM-chains, WalletKit-Dart depends on several JSON-RPC-providers as well as etherscan-styled APIs.
For EVM-chains, WalletKit-Dart depends on several JSON-RPC-providers as well as
etherscan-styled APIs.

## The following Unit-tests show how to use WalletKit-Dart

[Get Token Info for a ERC20 Token](https://github.com/nomo-app/walletkit-dart/blob/main/test/ci/evm/erc20_test.dart)

[Fetch EVM Transaction from Explorer](https://github.com/nomo-app/walletkit-dart/blob/main/test/ci/evm/evm_explorer_test.dart)
[Fetch EVM Transaction from Explorer](https://github.com/nomo-app/walletkit-dart/blob/main/test/ci/evm/evm_explorer_test.dart)

[Walletkit Json RPC Interface](https://github.com/nomo-app/walletkit-dart/blob/main/test/ci/evm/evm_rcp_test.dart)

Expand All @@ -126,5 +143,12 @@ For EVM-chains, WalletKit-Dart depends on several JSON-RPC-providers as well as

[Broadcast UTXO Transaction](https://github.com/nomo-app/walletkit-dart/blob/main/test/no_ci/wallet_test.dart)


protoc -I. --dart_out=grpc:/home/thomas/src/walletkit-dart/lib/src/crypto/tron/rpc core/Tron.proto core/Discover.proto core/TronInventoryItems.proto core/contract/common.proto core/contract/account_contract.proto core/contract/asset_issue_contract.proto core/contract/balance_contract.proto core/contract/exchange_contract.proto core/contract/market_contract.proto core/contract/proposal_contract.proto core/contract/shield_contract.proto core/contract/smart_contract.proto core/contract/storage_contract.proto core/contract/witness_contract.proto api/api.proto
protoc -I.
--dart_out=grpc:/home/thomas/src/walletkit-dart/lib/src/crypto/tron/rpc
core/Tron.proto core/Discover.proto core/TronInventoryItems.proto
core/contract/common.proto core/contract/account_contract.proto
core/contract/asset_issue_contract.proto core/contract/balance_contract.proto
core/contract/exchange_contract.proto core/contract/market_contract.proto
core/contract/proposal_contract.proto core/contract/shield_contract.proto
core/contract/smart_contract.proto core/contract/storage_contract.proto
core/contract/witness_contract.proto api/api.proto
Comment on lines +146 to +154
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove or properly document protoc commands.

The protoc commands at the end of the README appear to be development notes and include an absolute path. Consider either:

  1. Removing these commands if they're not meant for users
  2. Moving them to a development documentation file
  3. Converting them into a proper documentation section with context and relative paths

Loading