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

Jump to bottom

chore(test): re-enable can_store_after_restart test #2415

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore(test): re-enable can_store_after_restart test #2415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 15 additions & 2 deletions .github/workflows/merge.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,26 @@ jobs:
timeout-minutes: 25
run: cargo test --release --package sn_node --lib

# The `can_store_after_restart` can be executed with other package tests together and passing
# on local machine. However keeps failing (when executed together) on CI machines.
# This is most likely due to the setup and cocurrency issues of the tests.
# As the `record_store` is used in a single thread style, get the test passing executed
# and passing standalone is enough.
- name: Run network tests (with encrypt-records)
timeout-minutes: 25
run: cargo test --release --package sn_networking --features="open-metrics, encrypt-records"
run: cargo test --release --package sn_networking --features="open-metrics, encrypt-records" -- --skip can_store_after_restart

- name: Run network tests (with encrypt-records)
timeout-minutes: 5
run: cargo test --release --package sn_networking --features="open-metrics, encrypt-records" can_store_after_restart
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we run this test separately?

Copy link
Member Author

Choose a reason for hiding this comment

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

Strangely, I can run this combined with local machine.
However, the CI test always failed, which seems a parallel execution issue causing tests accessing same temp dir ?
Hence have to executed it separately.


- name: Run network tests (without encrypt-records)
timeout-minutes: 25
run: cargo test --release --package sn_networking --features="open-metrics"
run: cargo test --release --package sn_networking --features="open-metrics" -- --skip can_store_after_restart

- name: Run network tests (without encrypt-records)
timeout-minutes: 5
run: cargo test --release --package sn_networking --features="open-metrics" can_store_after_restart

- name: Run protocol tests
timeout-minutes: 25
Expand Down
17 changes: 11 additions & 6 deletions sn_networking/src/record_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,7 +1043,10 @@ mod tests {
use bls::SecretKey;
use xor_name::XorName;

use assert_fs::TempDir;
use assert_fs::{
fixture::{PathChild, PathCreateDir},
TempDir,
};
use bytes::Bytes;
use eyre::{bail, ContextCompat};
use libp2p::kad::K_VALUE;
Expand Down Expand Up @@ -1245,11 +1248,13 @@ mod tests {
}

#[tokio::test]
#[ignore = "fails on ci"]
async fn can_store_after_restart() -> eyre::Result<()> {
let temp_dir = TempDir::new().expect("Should be able to create a temp dir.");
let tmp_dir = TempDir::new()?;
let current_test_dir = tmp_dir.child("can_store_after_restart");
current_test_dir.create_dir_all()?;

let store_config = NodeRecordStoreConfig {
storage_dir: temp_dir.to_path_buf(),
storage_dir: current_test_dir.to_path_buf(),
encryption_seed: [1u8; 16],
..Default::default()
};
Expand Down Expand Up @@ -1290,7 +1295,7 @@ mod tests {
assert!(stored_record.is_some(), "Chunk should be stored");

// Sleep a while to let OS completes the flush to disk
sleep(Duration::from_secs(1)).await;
sleep(Duration::from_secs(5)).await;

// Restart the store with same encrypt_seed
drop(store);
Expand All @@ -1311,7 +1316,7 @@ mod tests {
// Restart the store with different encrypt_seed
let self_id_diff = PeerId::random();
let store_config_diff = NodeRecordStoreConfig {
storage_dir: temp_dir.to_path_buf(),
storage_dir: current_test_dir.to_path_buf(),
encryption_seed: [2u8; 16],
..Default::default()
};
Expand Down
Loading