Do not allow importing by test target name in integration tests (#1634)

software-mansion · Oct 11, 2024 · fd4f4ac · fd4f4ac
1 parent f088fe8
commit fd4f4ac
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 8 deletions.
diff --git a/scarb/src/compiler/db.rs b/scarb/src/compiler/db.rs
@@ -17,7 +17,7 @@ use tracing::trace;
 
 use crate::compiler::plugin::proc_macro::{ProcMacroHost, ProcMacroHostPlugin};
 use crate::compiler::{CairoCompilationUnit, CompilationUnitAttributes, CompilationUnitComponent};
-use crate::core::{ManifestDependency, Workspace};
+use crate::core::{ManifestDependency, TestTargetProps, TestTargetType, Workspace};
 use crate::DEFAULT_MODULE_MAIN_FILE;
 
 pub struct ScarbDatabase {
@@ -169,6 +169,7 @@ fn build_project_config(unit: &CairoCompilationUnit) -> Result<ProjectConfig> {
                         .any(|target| {
                             target.group_id.clone().unwrap_or(target.name.clone())
                                 == component.package.id.name.to_smol_str()
+                            && component_as_dependency.cairo_package_name() != component.cairo_package_name()
                         })
                 })
                 .map(|compilation_unit_component| {
@@ -184,13 +185,21 @@ fn build_project_config(unit: &CairoCompilationUnit) -> Result<ProjectConfig> {
                 .collect();
 
             // Adds itself to dependencies
-            dependencies.insert(
-                component.package.id.name.to_string(),
-                DependencySettings {
-                    version: (component.package.id.name.to_string() != *CORELIB_CRATE_NAME)
-                        .then_some(component.package.id.version.clone()),
-                },
-            );
+            let is_integration_test = if component.first_target().kind.is_test() {
+                let props: Option<TestTargetProps> = component.first_target().props().ok();
+                props
+                    .map(|props| props.test_type == TestTargetType::Integration)
+                    .unwrap_or_default()
+            } else { false };
+            if !is_integration_test {
+                dependencies.insert(
+                    component.package.id.name.to_string(),
+                    DependencySettings {
+                        version: (component.package.id.name.to_string() != *CORELIB_CRATE_NAME)
+                            .then_some(component.package.id.version.clone()),
+                    },
+                );
+            }
 
             (
                 component.cairo_package_name(),

diff --git a/scarb/tests/build.rs b/scarb/tests/build.rs
@@ -1513,3 +1513,37 @@ fn add_statements_code_locations_debug_info_to_tests() {
         "Expected statements_code_locations info to be a map"
     );
 }
+
+#[test]
+fn can_import_from_self_by_name() {
+    let t = TempDir::new().unwrap();
+    ProjectBuilder::start()
+        .name("hello")
+        .lib_cairo(indoc! {r#"
+            fn fib(mut n: u32) -> u32 {
+                let mut a: u32 = 0;
+                let mut b: u32 = 1;
+                while n != 0 {
+                    n = n - 1;
+                    let temp = b;
+                    b = a + b;
+                    a = temp;
+                };
+                a
+            }
+            
+            mod some {
+                use hello::fib;
+                
+                fn main() -> u32 {
+                    fib(16)
+                }
+            }
+        "#})
+        .build(&t);
+    Scarb::quick_snapbox()
+        .arg("build")
+        .current_dir(&t)
+        .assert()
+        .success();
+}
diff --git a/scarb/tests/build_targets.rs b/scarb/tests/build_targets.rs
@@ -417,6 +417,64 @@ fn integration_tests_do_not_enable_cfg_in_main_package() {
         "#});
 }
 
+#[test]
+fn integration_tests_cannot_use_itself_by_target_name() {
+    let t = TempDir::new().unwrap();
+    ProjectBuilder::start()
+        .name("hello")
+        .dep_cairo_test()
+        .lib_cairo(indoc! {r#"
+            fn hello_world() -> felt252 { 42 }
+        "#})
+        .build(&t);
+    t.child("tests").create_dir_all().unwrap();
+    t.child("tests/test1.cairo")
+        .write_str(indoc! {r#"
+        pub fn hello() -> felt252 { 12 }
+        pub fn beautiful() -> felt252 { 34 }
+        pub fn world() -> felt252 { 56 }
+
+        mod tests {
+            use hello_integrationtest::test1::world;
+            use hello_tests::test1::beautiful;
+            use crate::test1::hello;
+
+            #[test]
+            fn test_1() {
+                assert(world() == 12, '');
+            }
+        }
+        "#})
+        .unwrap();
+
+    Scarb::quick_snapbox()
+        .arg("build")
+        .arg("--test")
+        .current_dir(&t)
+        .assert()
+        .failure()
+        .stdout_matches(indoc! {r#"
+            [..]Compiling test(hello_unittest) hello v1.0.0 ([..]Scarb.toml)
+            [..]Compiling test(hello_integrationtest) hello_integrationtest v1.0.0 ([..]Scarb.toml)
+            error: Identifier not found.
+             --> [..]test1.cairo:6:9
+                use hello_integrationtest::test1::world;
+                    ^*******************^
+
+            error: Identifier not found.
+             --> [..]test1.cairo:7:9
+                use hello_tests::test1::beautiful;
+                    ^*********^
+
+            error: Type annotations needed. Failed to infer ?0.
+             --> [..]test1.cairo:12:16
+                    assert(world() == 12, '');
+                           ^***********^
+
+            error: could not compile `hello_integrationtest` due to previous error
+        "#});
+}
+
 #[test]
 fn detect_single_file_test_targets() {
     let t = TempDir::new().unwrap();