Merge #481

481: introduce marking infrastructure for maintainable tests r=matklad a=matklad



Co-authored-by: Aleksey Kladov <[email protected]>

4 files changed, 118 insertions, 1 deletions

diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index 1b6b72c98..b8246a7d1 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -17,6 +17,8 @@ macro_rules! ctry {
 pub mod db;
 #[cfg(test)]
 mod mock;
+#[macro_use]
+mod marks;
 mod query_definitions;
 mod path;
 pub mod source_binder;
diff --git a/crates/ra_hir/src/marks.rs b/crates/ra_hir/src/marks.rs
new file mode 100644
index 000000000..05430b975
--- /dev/null
+++ b/crates/ra_hir/src/marks.rs
@@ -0,0 +1,82 @@
+//! This module implements manually tracked test coverage, which useful for
+//! quickly finding a test responsible for testing a particular bit of code.
+//!
+//! See https://matklad.github.io/2018/06/18/a-trick-for-test-maintenance.html
+//! for details, but the TL;DR is that you write your test as
+//!
+//! ```no-run
+//! #[test]
+//! fn test_foo() {
+//!     covers!(test_foo);
+//! }
+//! ```
+//!
+//! and in the code under test you write
+//!
+//! ```no-run
+//! fn foo() {
+//!     if some_condition() {
+//!         tested_by!(test_foo);
+//!     }
+//! }
+//! ```
+//!
+//! This module then checks that executing the test indeed covers the specified
+//! function. This is useful if you come back to the `foo` function ten years
+//! later and wonder where the test are: now you can grep for `test_foo`.
+
+#[macro_export]
+macro_rules! tested_by {
+    ($ident:ident) => {
+        #[cfg(test)]
+        {
+            crate::marks::marks::$ident.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
+        }
+    };
+}
+
+#[macro_export]
+macro_rules! covers {
+    ($ident:ident) => {
+        let _checker = crate::marks::marks::MarkChecker::new(&crate::marks::marks::$ident);
+    };
+}
+
+#[cfg(test)]
+pub(crate) mod marks {
+    use std::sync::atomic::{AtomicUsize, Ordering};
+
+    pub(crate) struct MarkChecker {
+        mark: &'static AtomicUsize,
+        value_on_entry: usize,
+    }
+
+    impl MarkChecker {
+        pub(crate) fn new(mark: &'static AtomicUsize) -> MarkChecker {
+            let value_on_entry = mark.load(Ordering::SeqCst);
+            MarkChecker {
+                mark,
+                value_on_entry,
+            }
+        }
+    }
+
+    impl Drop for MarkChecker {
+        fn drop(&mut self) {
+            if std::thread::panicking() {
+                return;
+            }
+            let value_on_exit = self.mark.load(Ordering::SeqCst);
+            assert!(value_on_exit > self.value_on_entry, "mark was not hit")
+        }
+    }
+
+    macro_rules! mark {
+        ($ident:ident) => {
+            #[allow(bad_style)]
+            pub(crate) static $ident: AtomicUsize = AtomicUsize::new(0);
+        };
+    }
+
+    mark!(name_res_works_for_broken_modules);
+}
diff --git a/crates/ra_hir/src/module_tree.rs b/crates/ra_hir/src/module_tree.rs
index d2c92f150..50383c6d8 100644
--- a/crates/ra_hir/src/module_tree.rs
+++ b/crates/ra_hir/src/module_tree.rs
@@ -14,7 +14,7 @@ use ra_arena::{Arena, RawId, impl_arena_id};
 use crate::{Name, AsName, HirDatabase, SourceItemId, HirFileId, Problem, SourceFileItems, ModuleSource};
 
 impl ModuleSource {
-    pub fn from_source_item_id(
+    pub(crate) fn from_source_item_id(
         db: &impl HirDatabase,
         source_item_id: SourceItemId,
     ) -> ModuleSource {
@@ -217,6 +217,10 @@ fn modules(root: &impl ast::ModuleItemOwner) -> impl Iterator<Item = (Name, &ast
         })
         .filter_map(|module| {
             let name = module.name()?.as_name();
+            if !module.has_semi() && module.item_list().is_none() {
+                tested_by!(name_res_works_for_broken_modules);
+                return None;
+            }
             Some((name, module))
         })
 }
diff --git a/crates/ra_hir/src/nameres/tests.rs b/crates/ra_hir/src/nameres/tests.rs
index 17de54b44..ba9fcb3d1 100644
--- a/crates/ra_hir/src/nameres/tests.rs
+++ b/crates/ra_hir/src/nameres/tests.rs
@@ -137,6 +137,35 @@ fn re_exports() {
 }
 
 #[test]
+fn name_res_works_for_broken_modules() {
+    covers!(name_res_works_for_broken_modules);
+    let (item_map, module_id) = item_map(
+        "
+        //- /lib.rs
+        mod foo // no `;`, no body
+
+        use self::foo::Baz;
+        <|>
+
+        //- /foo/mod.rs
+        pub mod bar;
+
+        pub use self::bar::Baz;
+
+        //- /foo/bar.rs
+        pub struct Baz;
+    ",
+    );
+    check_module_item_map(
+        &item_map,
+        module_id,
+        "
+            Baz: _
+        ",
+    );
+}
+
+#[test]
 fn item_map_contains_items_from_expansions() {
     let (item_map, module_id) = item_map(
         "