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

chore(ci): minor fixes to improve CI workflow #460

Merged
merged 3 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
with:
toolchain: 1.81

- uses: Swatinem/rust-cache@v2 # use a different cache key as coverae uses custom rustc args
- uses: Swatinem/rust-cache@v2 # use a different cache key as coverage uses custom rustc args
with:
cache-provider: buildjet
key: "coverage"
Expand All @@ -49,7 +49,7 @@ jobs:
cargo build --bin madara --profile dev
export COVERAGE_BIN=$(realpath target/debug/madara)
rm -f target/madara-* lcov.info
cargo test --profile dev --workspace -- --test-threads=1
cargo test --profile dev --workspace

- name: Generate coverage info
run: |
Expand Down
10 changes: 7 additions & 3 deletions .github/workflows/db-version.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@ jobs:
update-db-version:
runs-on: ubuntu-latest
if: contains(github.event.pull_request.labels.*.name, 'db-migration')
permissions:
contents: write
steps:
- uses: actions/checkout@v3
with:
ref: ${{ github.head_ref || github.ref_name }}

- name: Install yq
run: sudo apt-get install -y yq
uses: mikefarah/yq@master

- name: Check if PR already bumped
id: check_bump
Expand All @@ -40,7 +44,7 @@ jobs:
if: steps.check_bump.outputs.already_bumped == 'false'
run: |
if [[ -n "$(git status --porcelain)" ]]; then
git add .db-versions.toml
git add .db-versions.yml
git commit -m "chore: bump db version"
git push origin HEAD
git push origin ${{ github.head_ref || github.ref_name }}
fi
8 changes: 4 additions & 4 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ concurrency:
cancel-in-progress: true

permissions:
contents: write
pull-requests: write

jobs:
Expand All @@ -22,7 +23,7 @@ jobs:
linters:
name: Run linters
needs: update_db_version
if: ${{ always() }}
if: ${{ github.event.pull_request.draft == false && always() }}
uses: ./.github/workflows/linters.yml

rust_check:
Expand All @@ -38,18 +39,17 @@ jobs:
coverage:
name: Run Coverage
needs: update_db_version
if: ${{ always() }}
if: ${{ github.event.pull_request.draft == false && always() }}
secrets: inherit
uses: ./.github/workflows/coverage.yml

build:
name: Build Madara
needs: update_db_version
if: ${{ always() }}
if: ${{ github.event.pull_request.draft == false && always() }}
uses: ./.github/workflows/build.yml

js_test:
name: Run JS Tests
needs: build
if: ${{ always() }}
uses: ./.github/workflows/starknet-js-test.yml
41 changes: 0 additions & 41 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ dotenv = "0.15.0"
httpmock = "0.7.0"
tempfile = "3.10.1"
mockall = "0.13.0"
serial_test = "3.1.1"
itertools = "0.13.0"
regex = "1.10.5"
bytes = "1.6.0"
Expand Down
1 change: 0 additions & 1 deletion crates/madara/client/eth/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ tempfile.workspace = true
dotenv.workspace = true
httpmock.workspace = true
tracing-test = "0.2.5"
serial_test.workspace = true
lazy_static.workspace = true
mp-utils = { workspace = true, features = ["testing"] }
mc-mempool = { workspace = true, features = ["testing"] }
44 changes: 31 additions & 13 deletions crates/madara/client/eth/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,7 @@ pub mod eth_client_getter_test {
primitives::U256,
};

use serial_test::serial;
use std::ops::Range;
use std::ops::{Deref, Range};
use std::sync::Mutex;
use tokio;

Expand Down Expand Up @@ -212,12 +211,37 @@ pub mod eth_client_getter_test {

static ANVIL: Mutex<Option<Arc<AnvilInstance>>> = Mutex::new(None);

pub fn get_shared_anvil() -> Arc<AnvilInstance> {
let mut anvil = ANVIL.lock().expect("poisoned lock");
if anvil.is_none() {
*anvil = Some(Arc::new(create_anvil_instance()));
/// Wrapper for an Anvil instance that automatically cleans up when all handles are dropped
pub struct AnvilHandle {
instance: Arc<AnvilInstance>,
}

impl Drop for AnvilHandle {
fn drop(&mut self) {
let mut guard = ANVIL.lock().expect("poisoned lock");
// Check if this Arc is the last one (strong_count == 2 because of our reference
// and the one in the static)
if Arc::strong_count(&self.instance) == 2 {
println!("Cleaning up Anvil instance");
*guard = None;
}
}
}

impl Deref for AnvilHandle {
type Target = AnvilInstance;

fn deref(&self) -> &Self::Target {
&self.instance
}
}

pub fn get_shared_anvil() -> AnvilHandle {
let mut guard = ANVIL.lock().expect("poisoned lock");
if guard.is_none() {
*guard = Some(Arc::new(create_anvil_instance()));
}
Arc::clone(anvil.as_ref().unwrap())
AnvilHandle { instance: Arc::clone(guard.as_ref().unwrap()) }
}

pub fn create_anvil_instance() -> AnvilInstance {
Expand Down Expand Up @@ -245,7 +269,6 @@ pub mod eth_client_getter_test {
EthereumClient { provider: Arc::new(provider), l1_core_contract: contract.clone(), l1_block_metrics }
}

#[serial]
#[tokio::test]
async fn fail_create_new_client_invalid_core_contract() {
let anvil = get_shared_anvil();
Expand All @@ -261,7 +284,6 @@ pub mod eth_client_getter_test {
assert!(new_client_result.is_err(), "EthereumClient::new should fail with an invalid core contract address");
}

#[serial]
#[tokio::test]
async fn get_latest_block_number_works() {
let anvil = get_shared_anvil();
Expand All @@ -271,7 +293,6 @@ pub mod eth_client_getter_test {
assert_eq!(block_number, L1_BLOCK_NUMBER, "provider unable to get the correct block number");
}

#[serial]
#[tokio::test]
async fn get_last_event_block_number_works() {
let anvil = get_shared_anvil();
Expand All @@ -283,7 +304,6 @@ pub mod eth_client_getter_test {
assert_eq!(block_number, L1_BLOCK_NUMBER, "block number with given event not matching");
}

#[serial]
#[tokio::test]
async fn get_last_verified_block_hash_works() {
let anvil = get_shared_anvil();
Expand All @@ -294,7 +314,6 @@ pub mod eth_client_getter_test {
assert_eq!(block_hash, expected, "latest block hash not matching");
}

#[serial]
#[tokio::test]
async fn get_last_state_root_works() {
let anvil = get_shared_anvil();
Expand All @@ -304,7 +323,6 @@ pub mod eth_client_getter_test {
assert_eq!(state_root, expected, "latest block state root not matching");
}

#[serial]
#[tokio::test]
async fn get_last_verified_block_number_works() {
let anvil = get_shared_anvil();
Expand Down
7 changes: 0 additions & 7 deletions crates/madara/client/eth/src/l1_gas_price.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,10 @@ mod eth_client_gas_price_worker_test {
use crate::client::eth_client_getter_test::{create_ethereum_client, get_shared_anvil};
use httpmock::{MockServer, Regex};
use mc_mempool::GasPriceProvider;
use serial_test::serial;
use std::time::SystemTime;
use tokio::task::JoinHandle;
use tokio::time::{timeout, Duration};

#[serial]
#[tokio::test]
async fn gas_price_worker_when_infinite_loop_true_works() {
let anvil = get_shared_anvil();
Expand Down Expand Up @@ -184,7 +182,6 @@ mod eth_client_gas_price_worker_test {
assert_eq!(updated_price.eth_l1_data_gas_price, 1);
}

#[serial]
#[tokio::test]
async fn gas_price_worker_when_infinite_loop_false_works() {
let anvil = get_shared_anvil();
Expand All @@ -203,7 +200,6 @@ mod eth_client_gas_price_worker_test {
assert_eq!(updated_price.eth_l1_data_gas_price, 1);
}

#[serial]
#[tokio::test]
async fn gas_price_worker_when_gas_price_fix_works() {
let anvil = get_shared_anvil();
Expand All @@ -224,7 +220,6 @@ mod eth_client_gas_price_worker_test {
assert_eq!(updated_price.eth_l1_data_gas_price, 1);
}

#[serial]
#[tokio::test]
async fn gas_price_worker_when_data_gas_price_fix_works() {
let anvil = get_shared_anvil();
Expand All @@ -245,7 +240,6 @@ mod eth_client_gas_price_worker_test {
assert_eq!(updated_price.eth_l1_data_gas_price, 20);
}

#[serial]
#[tokio::test]
async fn gas_price_worker_when_eth_fee_history_fails_should_fails() {
let mock_server = MockServer::start();
Expand Down Expand Up @@ -311,7 +305,6 @@ mod eth_client_gas_price_worker_test {
mock.assert();
}

#[serial]
#[tokio::test]
async fn update_gas_price_works() {
let anvil = get_shared_anvil();
Expand Down
6 changes: 5 additions & 1 deletion scripts/e2e-coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ cargo build --bin madara --profile dev
export COVERAGE_BIN=$(realpath target/debug/madara)

# Run tests with coverage collection
cargo test --profile dev "${@:-"--workspace"}"
if cargo test --profile dev "${@:-"--workspace"}"; then
echo "✅ All tests passed successfully!"
else
echo "❌ Some tests failed."
fi

# Generate coverage reports
cargo llvm-cov report --lcov --output-path lcov.info # Generate LCOV report
Expand Down
9 changes: 7 additions & 2 deletions scripts/e2e-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ export ETH_FORK_URL=https://eth.merkle.io

# Build the binary
cargo build --bin madara --profile dev
export BINARY_PATH=$(realpath target/debug/madara)
export COVERAGE_BIN=$(realpath target/debug/madara)

# Run the tests
cargo test --profile dev "${@:-"--workspace"}"
if cargo test --profile dev "${@:-"--workspace"}"; then
echo "✅ All tests passed successfully!"
else
echo "❌ Some tests failed."
exit 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why exit 1 for e2e tests but not coverage?

Copy link
Member Author

Choose a reason for hiding this comment

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

It depends on whether we still want the coverage report despite the test failures

fi
6 changes: 3 additions & 3 deletions scripts/update-db-version.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
#!/bin/bash

#
# Database version management script
Expand Down Expand Up @@ -64,7 +64,7 @@ CURRENT_VERSION=$(yq '.current_version' "$FILE")
NEW_VERSION=$((CURRENT_VERSION + 1))

# Update version and append to history
yq -i -y ".current_version = $NEW_VERSION |
.versions = [{\"version\": $NEW_VERSION, \"pr\": $PR_NUMBER}] + .versions" "$FILE"
yq e ".current_version = $NEW_VERSION |
.versions = [{\"version\": $NEW_VERSION, \"pr\": $PR_NUMBER}] + .versions" -i "$FILE"

echo "Successfully updated DB version to ${NEW_VERSION} (PR #${PR_NUMBER})"
Loading