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

Fix creating package with snfoundry test runner #1843

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+28 −8
Open

Fix creating package with snfoundry test runner #1843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04d07e1
Fix creating package with snfoundry test runner
ddoktorski Dec 11, 2024
d07f85e
Add snforge init test to CI workflow
ddoktorski Dec 17, 2024
8ced8d9
Run CI check only on ubuntu
ddoktorski Dec 17, 2024
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
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,11 @@ jobs:
cache-dependency-path: website/package-lock.json
- run: npm ci
- run: npm run fmt:check

snforge-init:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- uses: foundry-rs/setup-snfoundry@v3
- run: cargo test --profile=ci --package scarb --test snforge_init new_simple -- --ignored
10 changes: 5 additions & 5 deletions scarb/src/ops/new.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,12 @@ fn mk(
Ok(())
}

fn init_snforge(name: PackageName, target_dir: Utf8PathBuf, config: &Config) -> Result<()> {
let target_dir = target_dir.parent().context("package must have a parent")?;
fn init_snforge(name: PackageName, root_dir: Utf8PathBuf, config: &Config) -> Result<()> {
let mut process = Command::new("snforge")
.arg("init")
.arg(name.as_str())
.current_dir(target_dir)
.arg("new")
.args(["--name", name.as_str()])
.arg("--overwrite")
.arg(root_dir.as_str())
.envs(get_env_vars(config, None)?)
.stderr(Stdio::inherit())
.stdout(Stdio::inherit())
Expand Down
18 changes: 15 additions & 3 deletions scarb/tests/snforge_init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,23 @@ use assert_fs::TempDir;
use scarb::core::TomlManifest;
use scarb_test_support::command::Scarb;
use scarb_test_support::fsx::AssertFsUtf8Ext;
use test_case::test_case;

#[test]
#[test_case(None)]
#[test_case(Some("simple_project"))]
#[ignore = "run this test by name"]
fn new_simple() {
fn new_simple(package_name: Option<&str>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test fails with (when run with cargo test --profile=ci --package scarb --test snforge_init new_simple -- --ignored):

running 2 tests
test new_simple::none_expects ... FAILED
test new_simple::some_simple_project_expects ... FAILED

failures:

---- new_simple::none_expects stdout ----
thread 'new_simple::none_expects' panicked at scarb/tests/snforge_init.rs:34:5:
assertion failed: t.child("tests").is_dir()

---- new_simple::some_simple_project_expects stdout ----
thread 'new_simple::some_simple_project_expects' panicked at scarb/tests/snforge_init.rs:34:5:
assertion failed: t.child("tests").is_dir()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    new_simple::none_expects
    new_simple::some_simple_project_expects

test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.58s

Note it's currently not run as part of our CI, it's only run once per day from main and during release ->

scarb/.github/workflows/_check-release.yml

Lines 44 to 50 in 939c7f1

snforge-init:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- uses: foundry-rs/setup-snfoundry@v3
- run: cargo test --profile=ci --package scarb --test snforge_init new_simple -- --ignored

Would you mind adding it to the CI pipeline as well? 🙏 https://github.com/software-mansion/scarb/blob/main/.github/workflows/ci.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be fixed by the next foundry release, right?

Copy link
Author

Choose a reason for hiding this comment

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

This test fails with (when run with cargo test --profile=ci --package scarb --test snforge_init new_simple -- --ignored)

Are you sure you have the latest snfoundry version installed (0.35.0)? It was released yesterday

Would you mind adding it to the CI pipeline as well?

Can we simply remove #[ignore] so it can be run with all other tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we simply remove #[ignore] so it can be run with all other tests?

The other tests run without Foundry installed.

let pt = TempDir::new().unwrap();

let name_args = if let Some(package_name) = package_name {
vec!["--name", package_name]
} else {
vec![]
};

Scarb::quick_snapbox()
.arg("new")
.args(name_args)
.arg("hello")
.args(["--test-runner", "starknet-foundry"])
.current_dir(&pt)
Expand All @@ -27,7 +36,10 @@ fn new_simple() {
assert!(t.child(".git").is_dir());

let toml_manifest = TomlManifest::read_from_path(t.child("Scarb.toml").utf8_path()).unwrap();
assert_eq!(toml_manifest.package.unwrap().name.as_str(), "hello");
assert_eq!(
toml_manifest.package.unwrap().name.as_str(),
package_name.unwrap_or("hello")
);
let deps = toml_manifest.dependencies.unwrap();
assert_eq!(deps.len(), 1);
assert!(deps.contains_key("starknet"));
Expand Down
Loading