From 65536f872b4ab854509f36942c775c65f60f6b25 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 18 Dec 2018 13:40:50 +0100 Subject: [PATCH 1/6] Split tooltip tests whether they depend on Racer fallback --- src/actions/hover.rs | 87 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 10 deletions(-) diff --git a/src/actions/hover.rs b/src/actions/hover.rs index a7df9109807..8efa6c87c6f 100644 --- a/src/actions/hover.rs +++ b/src/actions/hover.rs @@ -1175,7 +1175,11 @@ pub mod test { impl TooltipTestHarness { /// Creates a new `TooltipTestHarness`. The `project_dir` must contain /// a valid rust project with a `Cargo.toml`. - pub fn new(project_dir: PathBuf, output: &O) -> TooltipTestHarness { + pub fn new( + project_dir: PathBuf, + output: &O, + racer_fallback_completion: bool, + ) -> TooltipTestHarness { use env_logger; let _ = env_logger::try_init(); @@ -1194,6 +1198,7 @@ pub mod test { let temp_dir = tempfile::tempdir().unwrap().into_path(); config.target_dir = config::Inferrable::Specified(Some(temp_dir.clone())); + config.racer_completion = racer_fallback_completion; let config = Arc::new(Mutex::new(config)); let analysis = Arc::new(analysis::AnalysisHost::new(analysis::Target::Debug)); @@ -2082,7 +2087,6 @@ pub mod test { Test::new("test_tooltip_01.rs", 68, 11), Test::new("test_tooltip_01.rs", 68, 26), Test::new("test_tooltip_01.rs", 75, 10), - Test::new("test_tooltip_01.rs", 80, 11), Test::new("test_tooltip_01.rs", 85, 14), Test::new("test_tooltip_01.rs", 85, 50), Test::new("test_tooltip_01.rs", 85, 54), @@ -2091,7 +2095,6 @@ pub mod test { Test::new("test_tooltip_01.rs", 87, 20), Test::new("test_tooltip_01.rs", 88, 18), Test::new("test_tooltip_01.rs", 93, 11), - Test::new("test_tooltip_01.rs", 93, 18), Test::new("test_tooltip_01.rs", 95, 25), Test::new("test_tooltip_01.rs", 109, 21), Test::new("test_tooltip_01.rs", 113, 21), @@ -2100,7 +2103,38 @@ pub mod test { Test::new("test_tooltip_mod_use.rs", 12, 14), Test::new("test_tooltip_mod_use.rs", 12, 25), Test::new("test_tooltip_mod_use.rs", 13, 28), - Test::new("test_tooltip_mod_use_external.rs", 11, 7), + ]; + + let out = LineOutput::default(); + let proj_dir = FIXTURES_DIR.join("hover"); + + let save_dir_guard = tempfile::tempdir().unwrap(); + let save_dir = save_dir_guard.path().to_owned(); + let load_dir = proj_dir.join("save_data"); + + let harness = TooltipTestHarness::new(proj_dir, &out, false); + + out.reset(); + + let failures = harness.run_tests(&tests, load_dir, save_dir)?; + + if failures.is_empty() { + Ok(()) + } else { + eprintln!("{}\n\n", out.reset().join("\n")); + eprintln!("Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", failures); + Err(format!("{} of {} tooltip tests failed", failures.len(), tests.len()).into()) + } + } + + #[test] + fn test_tooltip_racer() -> Result<(), Box> { + let _ = env_logger::try_init(); + use self::test::{LineOutput, Test, TooltipTestHarness}; + + let tests = vec![ + Test::new("test_tooltip_01.rs", 80, 11), + Test::new("test_tooltip_01.rs", 93, 18), Test::new("test_tooltip_mod_use_external.rs", 11, 7), Test::new("test_tooltip_mod_use_external.rs", 12, 7), Test::new("test_tooltip_mod_use_external.rs", 12, 12), @@ -2113,7 +2147,7 @@ pub mod test { let save_dir = save_dir_guard.path().to_owned(); let load_dir = proj_dir.join("save_data"); - let harness = TooltipTestHarness::new(proj_dir, &out); + let harness = TooltipTestHarness::new(proj_dir, &out, true); out.reset(); @@ -2138,10 +2172,6 @@ pub mod test { use self::test::{LineOutput, Test, TooltipTestHarness}; let tests = vec![ - // these test std stuff - Test::new("test_tooltip_mod_use_external.rs", 14, 12), - Test::new("test_tooltip_mod_use_external.rs", 15, 12), - Test::new("test_tooltip_std.rs", 18, 15), Test::new("test_tooltip_std.rs", 18, 27), Test::new("test_tooltip_std.rs", 19, 7), @@ -2164,7 +2194,44 @@ pub mod test { let save_dir = save_dir_guard.path().to_owned(); let load_dir = proj_dir.join("save_data"); - let harness = TooltipTestHarness::new(proj_dir, &out); + let harness = TooltipTestHarness::new(proj_dir, &out, false); + + out.reset(); + + let failures = harness.run_tests(&tests, load_dir, save_dir)?; + + if failures.is_empty() { + Ok(()) + } else { + eprintln!("{}\n\n", out.reset().join("\n")); + eprintln!("Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", failures); + Err(format!("{} of {} tooltip tests failed", failures.len(), tests.len()).into()) + } + } + + /// Note: This test is ignored as it doesn't work in the rust-lang/rust repo. + /// It is enabled on CI. + /// Run with `cargo test test_tooltip_std -- --ignored` + #[test] + #[ignore] + fn test_tooltip_std_racer() -> Result<(), Box> { + let _ = env_logger::try_init(); + use self::test::{LineOutput, Test, TooltipTestHarness}; + + let tests = vec![ + // these test std stuff + Test::new("test_tooltip_mod_use_external.rs", 14, 12), + Test::new("test_tooltip_mod_use_external.rs", 15, 12), + ]; + + let out = LineOutput::default(); + let proj_dir = FIXTURES_DIR.join("hover"); + + let save_dir_guard = tempfile::tempdir().unwrap(); + let save_dir = save_dir_guard.path().to_owned(); + let load_dir = proj_dir.join("save_data"); + + let harness = TooltipTestHarness::new(proj_dir, &out, true); out.reset(); From fb80e67759940b3287acb314da21a2f7f80921eb Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 18 Dec 2018 14:06:52 +0100 Subject: [PATCH 2/6] Separate common setup logic for test_tooltip tests --- src/actions/hover.rs | 111 +++++++++++-------------------------------- 1 file changed, 27 insertions(+), 84 deletions(-) diff --git a/src/actions/hover.rs b/src/actions/hover.rs index 8efa6c87c6f..eb6cd5bce38 100644 --- a/src/actions/hover.rs +++ b/src/actions/hover.rs @@ -2052,10 +2052,32 @@ pub mod test { assert_eq!(expected, actual); } + // Common logic used in `test_tooltip_*` tests below + fn run_tooltip_tests(tests: &[Test], proj_dir: PathBuf, racer_completion: bool) -> Result<(), Box> { + let out = LineOutput::default(); + + let save_dir_guard = tempfile::tempdir().unwrap(); + let save_dir = save_dir_guard.path().to_owned(); + let load_dir = proj_dir.join("save_data"); + + let harness = TooltipTestHarness::new(proj_dir, &out, racer_completion); + + out.reset(); + + let failures = harness.run_tests(tests, load_dir, save_dir)?; + + if failures.is_empty() { + Ok(()) + } else { + eprintln!("{}\n\n", out.reset().join("\n")); + eprintln!("Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", failures); + Err(format!("{} of {} tooltip tests failed", failures.len(), tests.len()).into()) + } + } + #[test] fn test_tooltip() -> Result<(), Box> { let _ = env_logger::try_init(); - use self::test::{LineOutput, Test, TooltipTestHarness}; let tests = vec![ Test::new("test_tooltip_01.rs", 13, 11), @@ -2105,32 +2127,12 @@ pub mod test { Test::new("test_tooltip_mod_use.rs", 13, 28), ]; - let out = LineOutput::default(); - let proj_dir = FIXTURES_DIR.join("hover"); - - let save_dir_guard = tempfile::tempdir().unwrap(); - let save_dir = save_dir_guard.path().to_owned(); - let load_dir = proj_dir.join("save_data"); - - let harness = TooltipTestHarness::new(proj_dir, &out, false); - - out.reset(); - - let failures = harness.run_tests(&tests, load_dir, save_dir)?; - - if failures.is_empty() { - Ok(()) - } else { - eprintln!("{}\n\n", out.reset().join("\n")); - eprintln!("Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", failures); - Err(format!("{} of {} tooltip tests failed", failures.len(), tests.len()).into()) - } + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), false) } #[test] fn test_tooltip_racer() -> Result<(), Box> { let _ = env_logger::try_init(); - use self::test::{LineOutput, Test, TooltipTestHarness}; let tests = vec![ Test::new("test_tooltip_01.rs", 80, 11), @@ -2140,26 +2142,7 @@ pub mod test { Test::new("test_tooltip_mod_use_external.rs", 12, 12), ]; - let out = LineOutput::default(); - let proj_dir = FIXTURES_DIR.join("hover"); - - let save_dir_guard = tempfile::tempdir().unwrap(); - let save_dir = save_dir_guard.path().to_owned(); - let load_dir = proj_dir.join("save_data"); - - let harness = TooltipTestHarness::new(proj_dir, &out, true); - - out.reset(); - - let failures = harness.run_tests(&tests, load_dir, save_dir)?; - - if failures.is_empty() { - Ok(()) - } else { - eprintln!("{}\n\n", out.reset().join("\n")); - eprintln!("Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", failures); - Err(format!("{} of {} tooltip tests failed", failures.len(), tests.len()).into()) - } + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), true) } /// Note: This test is ignored as it doesn't work in the rust-lang/rust repo. @@ -2169,7 +2152,6 @@ pub mod test { #[ignore] fn test_tooltip_std() -> Result<(), Box> { let _ = env_logger::try_init(); - use self::test::{LineOutput, Test, TooltipTestHarness}; let tests = vec![ Test::new("test_tooltip_std.rs", 18, 15), @@ -2187,26 +2169,7 @@ pub mod test { Test::new("test_tooltip_std.rs", 25, 25), ]; - let out = LineOutput::default(); - let proj_dir = FIXTURES_DIR.join("hover"); - - let save_dir_guard = tempfile::tempdir().unwrap(); - let save_dir = save_dir_guard.path().to_owned(); - let load_dir = proj_dir.join("save_data"); - - let harness = TooltipTestHarness::new(proj_dir, &out, false); - - out.reset(); - - let failures = harness.run_tests(&tests, load_dir, save_dir)?; - - if failures.is_empty() { - Ok(()) - } else { - eprintln!("{}\n\n", out.reset().join("\n")); - eprintln!("Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", failures); - Err(format!("{} of {} tooltip tests failed", failures.len(), tests.len()).into()) - } + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), false) } /// Note: This test is ignored as it doesn't work in the rust-lang/rust repo. @@ -2216,7 +2179,6 @@ pub mod test { #[ignore] fn test_tooltip_std_racer() -> Result<(), Box> { let _ = env_logger::try_init(); - use self::test::{LineOutput, Test, TooltipTestHarness}; let tests = vec![ // these test std stuff @@ -2224,25 +2186,6 @@ pub mod test { Test::new("test_tooltip_mod_use_external.rs", 15, 12), ]; - let out = LineOutput::default(); - let proj_dir = FIXTURES_DIR.join("hover"); - - let save_dir_guard = tempfile::tempdir().unwrap(); - let save_dir = save_dir_guard.path().to_owned(); - let load_dir = proj_dir.join("save_data"); - - let harness = TooltipTestHarness::new(proj_dir, &out, true); - - out.reset(); - - let failures = harness.run_tests(&tests, load_dir, save_dir)?; - - if failures.is_empty() { - Ok(()) - } else { - eprintln!("{}\n\n", out.reset().join("\n")); - eprintln!("Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", failures); - Err(format!("{} of {} tooltip tests failed", failures.len(), tests.len()).into()) - } + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), true) } } From 389832456fcb97d6724359bbaf937a4e76c73d33 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 18 Dec 2018 14:51:27 +0100 Subject: [PATCH 3/6] Use Racer enum for bool argument --- src/actions/hover.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/actions/hover.rs b/src/actions/hover.rs index eb6cd5bce38..d05c09359fc 100644 --- a/src/actions/hover.rs +++ b/src/actions/hover.rs @@ -2052,15 +2052,29 @@ pub mod test { assert_eq!(expected, actual); } + enum Racer { + Yes, + No, + } + + impl Into for Racer { + fn into(self) -> bool { + match self { + Racer::Yes => true, + Racer::No => false, + } + } + } + // Common logic used in `test_tooltip_*` tests below - fn run_tooltip_tests(tests: &[Test], proj_dir: PathBuf, racer_completion: bool) -> Result<(), Box> { + fn run_tooltip_tests(tests: &[Test], proj_dir: PathBuf, racer_completion: Racer) -> Result<(), Box> { let out = LineOutput::default(); let save_dir_guard = tempfile::tempdir().unwrap(); let save_dir = save_dir_guard.path().to_owned(); let load_dir = proj_dir.join("save_data"); - let harness = TooltipTestHarness::new(proj_dir, &out, racer_completion); + let harness = TooltipTestHarness::new(proj_dir, &out, racer_completion.into()); out.reset(); @@ -2127,7 +2141,7 @@ pub mod test { Test::new("test_tooltip_mod_use.rs", 13, 28), ]; - run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), false) + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), Racer::No) } #[test] @@ -2142,7 +2156,7 @@ pub mod test { Test::new("test_tooltip_mod_use_external.rs", 12, 12), ]; - run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), true) + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), Racer::Yes) } /// Note: This test is ignored as it doesn't work in the rust-lang/rust repo. @@ -2169,7 +2183,7 @@ pub mod test { Test::new("test_tooltip_std.rs", 25, 25), ]; - run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), false) + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), Racer::No) } /// Note: This test is ignored as it doesn't work in the rust-lang/rust repo. @@ -2186,6 +2200,6 @@ pub mod test { Test::new("test_tooltip_mod_use_external.rs", 15, 12), ]; - run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), true) + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), Racer::Yes) } } From 17b99622b6d7cd4e995df4299f8cf1ef509092e5 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 18 Dec 2018 14:56:45 +0100 Subject: [PATCH 4/6] Apply formatting to run_tooltip_tests --- src/actions/hover.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/actions/hover.rs b/src/actions/hover.rs index d05c09359fc..67719eaf921 100644 --- a/src/actions/hover.rs +++ b/src/actions/hover.rs @@ -2067,7 +2067,11 @@ pub mod test { } // Common logic used in `test_tooltip_*` tests below - fn run_tooltip_tests(tests: &[Test], proj_dir: PathBuf, racer_completion: Racer) -> Result<(), Box> { + fn run_tooltip_tests( + tests: &[Test], + proj_dir: PathBuf, + racer_completion: Racer, + ) -> Result<(), Box> { let out = LineOutput::default(); let save_dir_guard = tempfile::tempdir().unwrap(); @@ -2084,7 +2088,10 @@ pub mod test { Ok(()) } else { eprintln!("{}\n\n", out.reset().join("\n")); - eprintln!("Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", failures); + eprintln!( + "Failures (\x1b[91mexpected\x1b[92mactual\x1b[0m): {:#?}\n\n", + failures + ); Err(format!("{} of {} tooltip tests failed", failures.len(), tests.len()).into()) } } From 7f04b38eb83ecffe6d34b1066dde2f9a8caeb984 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 18 Dec 2018 23:09:53 +0100 Subject: [PATCH 5/6] Fix style/clarity for Racer enum --- src/actions/hover.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/actions/hover.rs b/src/actions/hover.rs index 67719eaf921..ab966b5953b 100644 --- a/src/actions/hover.rs +++ b/src/actions/hover.rs @@ -2052,16 +2052,16 @@ pub mod test { assert_eq!(expected, actual); } - enum Racer { + enum RacerFallback { Yes, No, } - impl Into for Racer { - fn into(self) -> bool { - match self { - Racer::Yes => true, - Racer::No => false, + impl From for bool { + fn from(arg: RacerFallback) -> bool { + match arg { + RacerFallback::Yes => true, + RacerFallback::No => false, } } } @@ -2070,7 +2070,7 @@ pub mod test { fn run_tooltip_tests( tests: &[Test], proj_dir: PathBuf, - racer_completion: Racer, + racer_completion: RacerFallback, ) -> Result<(), Box> { let out = LineOutput::default(); @@ -2148,7 +2148,7 @@ pub mod test { Test::new("test_tooltip_mod_use.rs", 13, 28), ]; - run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), Racer::No) + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), RacerFallback::No) } #[test] @@ -2163,7 +2163,7 @@ pub mod test { Test::new("test_tooltip_mod_use_external.rs", 12, 12), ]; - run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), Racer::Yes) + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), RacerFallback::Yes) } /// Note: This test is ignored as it doesn't work in the rust-lang/rust repo. @@ -2190,7 +2190,7 @@ pub mod test { Test::new("test_tooltip_std.rs", 25, 25), ]; - run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), Racer::No) + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), RacerFallback::No) } /// Note: This test is ignored as it doesn't work in the rust-lang/rust repo. @@ -2207,6 +2207,6 @@ pub mod test { Test::new("test_tooltip_mod_use_external.rs", 15, 12), ]; - run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), Racer::Yes) + run_tooltip_tests(&tests, FIXTURES_DIR.join("hover"), RacerFallback::Yes) } } From 632f21a17a194f9be51e0d80664cb38ade168452 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Wed, 19 Dec 2018 01:55:59 +0100 Subject: [PATCH 6/6] Disable `all_targets` in tooltip test runs More details here: https://github.com/rust-lang/rls/issues/1151#issuecomment-448415490 --- src/actions/hover.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/actions/hover.rs b/src/actions/hover.rs index ab966b5953b..7ed49cd4162 100644 --- a/src/actions/hover.rs +++ b/src/actions/hover.rs @@ -1199,6 +1199,10 @@ pub mod test { let temp_dir = tempfile::tempdir().unwrap().into_path(); config.target_dir = config::Inferrable::Specified(Some(temp_dir.clone())); config.racer_completion = racer_fallback_completion; + // FIXME(#1195): This led to spurious failures on macOS; possibly + // because regular build and #[cfg(test)] did race or rls-analysis + // didn't lower them properly? + config.all_targets = false; let config = Arc::new(Mutex::new(config)); let analysis = Arc::new(analysis::AnalysisHost::new(analysis::Target::Debug));