Igor Gnatenko
7 years ago
parent
3290808359
commit
b77b3cb996
@ -0,0 +1,68 @@
|
|||||||
|
From c66f7857e36102cdb7a2518650d5053b7d55e467 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Andrew Gallant <jamslam@gmail.com>
|
||||||
|
Date: Sun, 29 Jul 2018 09:40:38 -0400
|
||||||
|
Subject: [PATCH 1/2] tests/style: 80 columns, dammit
|
||||||
|
|
||||||
|
(cherry picked from commit 7c412bb2fa343a8d54090ea175c851cd822d8f62)
|
||||||
|
---
|
||||||
|
tests/workdir.rs | 23 +++++++++++++++++++----
|
||||||
|
1 file changed, 19 insertions(+), 4 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/tests/workdir.rs b/tests/workdir.rs
|
||||||
|
index 3c47e94..a78f70d 100644
|
||||||
|
--- a/tests/workdir.rs
|
||||||
|
+++ b/tests/workdir.rs
|
||||||
|
@@ -49,7 +49,11 @@ impl WorkDir {
|
||||||
|
|
||||||
|
/// Try to create a new file with the given name and contents in this
|
||||||
|
/// directory.
|
||||||
|
- pub fn try_create<P: AsRef<Path>>(&self, name: P, contents: &str) -> io::Result<()> {
|
||||||
|
+ pub fn try_create<P: AsRef<Path>>(
|
||||||
|
+ &self,
|
||||||
|
+ name: P,
|
||||||
|
+ contents: &str,
|
||||||
|
+ ) -> io::Result<()> {
|
||||||
|
let path = self.dir.join(name);
|
||||||
|
self.try_create_bytes(path, contents.as_bytes())
|
||||||
|
}
|
||||||
|
@@ -70,7 +74,11 @@ impl WorkDir {
|
||||||
|
|
||||||
|
/// Try to create a new file with the given name and contents in this
|
||||||
|
/// directory.
|
||||||
|
- fn try_create_bytes<P: AsRef<Path>>(&self, path: P, contents: &[u8]) -> io::Result<()> {
|
||||||
|
+ fn try_create_bytes<P: AsRef<Path>>(
|
||||||
|
+ &self,
|
||||||
|
+ path: P,
|
||||||
|
+ contents: &[u8],
|
||||||
|
+ ) -> io::Result<()> {
|
||||||
|
let mut file = File::create(&path)?;
|
||||||
|
file.write_all(contents)?;
|
||||||
|
file.flush()
|
||||||
|
@@ -190,7 +198,11 @@ impl WorkDir {
|
||||||
|
match stdout.parse() {
|
||||||
|
Ok(t) => t,
|
||||||
|
Err(err) => {
|
||||||
|
- panic!("could not convert from string: {:?}\n\n{}", err, stdout);
|
||||||
|
+ panic!(
|
||||||
|
+ "could not convert from string: {:?}\n\n{}",
|
||||||
|
+ err,
|
||||||
|
+ stdout
|
||||||
|
+ );
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@@ -221,7 +233,10 @@ impl WorkDir {
|
||||||
|
write!(stdin, "{}", input)
|
||||||
|
});
|
||||||
|
|
||||||
|
- let output = self.expect_success(cmd, child.wait_with_output().unwrap());
|
||||||
|
+ let output = self.expect_success(
|
||||||
|
+ cmd,
|
||||||
|
+ child.wait_with_output().unwrap(),
|
||||||
|
+ );
|
||||||
|
worker.join().unwrap().unwrap();
|
||||||
|
output
|
||||||
|
}
|
||||||
|
--
|
||||||
|
2.18.0
|
||||||
|
|
||||||
@ -0,0 +1,177 @@
|
|||||||
|
From 7f8d42ff8e2e73f302c97589ce6ff5beeb36051c Mon Sep 17 00:00:00 2001
|
||||||
|
From: Andrew Gallant <jamslam@gmail.com>
|
||||||
|
Date: Sun, 29 Jul 2018 10:15:20 -0400
|
||||||
|
Subject: [PATCH 2/2] tests: reduce reliance on state in tests
|
||||||
|
|
||||||
|
This commit improves the integration test setup by running tests inside
|
||||||
|
the system's temporary directory instead of within ripgrep's `target`
|
||||||
|
directory. The motivation here is to attempt to reduce the effect of
|
||||||
|
unanticipated state on ripgrep's integration tests, such as the presence
|
||||||
|
of `.gitignore` files in ripgrep's checkout directory hierarchy
|
||||||
|
(including parent directories).
|
||||||
|
|
||||||
|
This doesn't remove all possible state. For example, there's no
|
||||||
|
guarantee that the system's temporary directory isn't itself within a
|
||||||
|
git repository. Moreover, there may still be other ignore rules in the
|
||||||
|
directory tree that might impact test behavior. Fixing this seems
|
||||||
|
somewhat difficult. Conceptually, it seems like ripgrep should run each
|
||||||
|
test in its own `chroot`-like environment, but doing this in a
|
||||||
|
non-annoying and portable way (including Windows) doesn't appear to be
|
||||||
|
possible.
|
||||||
|
|
||||||
|
Another approach to take here might be to teach ripgrep itself that a
|
||||||
|
particular directory should be treated as root, and therefore, never
|
||||||
|
look at anything outside that directory. This also seems complex to
|
||||||
|
implement, but tractable. Let's see how this approach works for now.
|
||||||
|
|
||||||
|
Fixes #448, #996
|
||||||
|
|
||||||
|
(cherry picked from commit 2913fc4cd063f4d869f54497a313aafbf5330346)
|
||||||
|
---
|
||||||
|
tests/tests.rs | 9 +++++++++
|
||||||
|
tests/workdir.rs | 36 +++++++++++++-----------------------
|
||||||
|
2 files changed, 22 insertions(+), 23 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/tests/tests.rs b/tests/tests.rs
|
||||||
|
index 34bf08e..9569c07 100644
|
||||||
|
--- a/tests/tests.rs
|
||||||
|
+++ b/tests/tests.rs
|
||||||
|
@@ -575,6 +575,7 @@ sherlock!(no_ignore_hidden, "Sherlock", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
});
|
||||||
|
|
||||||
|
sherlock!(ignore_git, "Sherlock", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", "sherlock\n");
|
||||||
|
wd.assert_err(&mut cmd);
|
||||||
|
});
|
||||||
|
@@ -774,6 +775,7 @@ sherlock:5:12:but Doctor Watson has to have it taken out for him and dusted,
|
||||||
|
|
||||||
|
// See: https://github.com/BurntSushi/ripgrep/issues/16
|
||||||
|
clean!(regression_16, "xyz", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", "ghi/");
|
||||||
|
wd.create_dir("ghi");
|
||||||
|
wd.create_dir("def/ghi");
|
||||||
|
@@ -833,6 +835,7 @@ clean!(regression_50, "xyz", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
|
||||||
|
// See: https://github.com/BurntSushi/ripgrep/issues/65
|
||||||
|
clean!(regression_65, "xyz", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", "a/");
|
||||||
|
wd.create_dir("a");
|
||||||
|
wd.create("a/foo", "xyz");
|
||||||
|
@@ -842,6 +845,7 @@ clean!(regression_65, "xyz", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
|
||||||
|
// See: https://github.com/BurntSushi/ripgrep/issues/67
|
||||||
|
clean!(regression_67, "test", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", "/*\n!/dir");
|
||||||
|
wd.create_dir("dir");
|
||||||
|
wd.create_dir("foo");
|
||||||
|
@@ -854,6 +858,7 @@ clean!(regression_67, "test", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
|
||||||
|
// See: https://github.com/BurntSushi/ripgrep/issues/87
|
||||||
|
clean!(regression_87, "test", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", "foo\n**no-vcs**");
|
||||||
|
wd.create("foo", "test");
|
||||||
|
wd.assert_err(&mut cmd);
|
||||||
|
@@ -861,6 +866,7 @@ clean!(regression_87, "test", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
|
||||||
|
// See: https://github.com/BurntSushi/ripgrep/issues/90
|
||||||
|
clean!(regression_90, "test", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", "!.foo");
|
||||||
|
wd.create(".foo", "test");
|
||||||
|
|
||||||
|
@@ -921,6 +927,7 @@ clean!(regression_127, "Sherlock", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
// ripgrep should ignore 'foo/sherlock' giving us results only from
|
||||||
|
// 'foo/watson' but on Windows ripgrep will include both 'foo/sherlock' and
|
||||||
|
// 'foo/watson' in the search results.
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", "foo/sherlock\n");
|
||||||
|
wd.create_dir("foo");
|
||||||
|
wd.create("foo/sherlock", hay::SHERLOCK);
|
||||||
|
@@ -948,6 +955,7 @@ clean!(regression_128, "x", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
// TODO(burntsushi): Darwin doesn't like this test for some reason.
|
||||||
|
#[cfg(not(target_os = "macos"))]
|
||||||
|
clean!(regression_131, "test", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", "TopÑapa");
|
||||||
|
wd.create("TopÑapa", "test");
|
||||||
|
wd.assert_err(&mut cmd);
|
||||||
|
@@ -1235,6 +1243,7 @@ clean!(regression_599, "^$", "input.txt", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
|
||||||
|
// See: https://github.com/BurntSushi/ripgrep/issues/807
|
||||||
|
clean!(regression_807, "test", ".", |wd: WorkDir, mut cmd: Command| {
|
||||||
|
+ wd.create_dir(".git");
|
||||||
|
wd.create(".gitignore", ".a/b");
|
||||||
|
wd.create_dir(".a/b");
|
||||||
|
wd.create_dir(".a/c");
|
||||||
|
diff --git a/tests/workdir.rs b/tests/workdir.rs
|
||||||
|
index a78f70d..9736a64 100644
|
||||||
|
--- a/tests/workdir.rs
|
||||||
|
+++ b/tests/workdir.rs
|
||||||
|
@@ -21,7 +21,8 @@ pub struct WorkDir {
|
||||||
|
/// The directory in which this test executable is running.
|
||||||
|
root: PathBuf,
|
||||||
|
/// The directory in which the test should run. If a test needs to create
|
||||||
|
- /// files, they should go in here.
|
||||||
|
+ /// files, they should go in here. This directory is also used as the CWD
|
||||||
|
+ /// for any processes created by the test.
|
||||||
|
dir: PathBuf,
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -31,9 +32,15 @@ impl WorkDir {
|
||||||
|
/// to a logical grouping of tests.
|
||||||
|
pub fn new(name: &str) -> WorkDir {
|
||||||
|
let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);
|
||||||
|
- let root = env::current_exe().unwrap()
|
||||||
|
- .parent().expect("executable's directory").to_path_buf();
|
||||||
|
- let dir = root.join(TEST_DIR).join(name).join(&format!("{}", id));
|
||||||
|
+ let root = env::current_exe()
|
||||||
|
+ .unwrap()
|
||||||
|
+ .parent()
|
||||||
|
+ .expect("executable's directory")
|
||||||
|
+ .to_path_buf();
|
||||||
|
+ let dir = env::temp_dir()
|
||||||
|
+ .join(TEST_DIR)
|
||||||
|
+ .join(name)
|
||||||
|
+ .join(&format!("{}", id));
|
||||||
|
nice_err(&dir, repeat(|| fs::create_dir_all(&dir)));
|
||||||
|
WorkDir {
|
||||||
|
root: root,
|
||||||
|
@@ -107,28 +114,11 @@ impl WorkDir {
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Returns the path to the ripgrep executable.
|
||||||
|
- #[cfg(not(windows))]
|
||||||
|
- pub fn bin(&self) -> PathBuf {
|
||||||
|
- let path = self.root.join("rg");
|
||||||
|
- if !path.is_file() {
|
||||||
|
- // Looks like a recent version of Cargo changed the cwd or the
|
||||||
|
- // location of the test executable.
|
||||||
|
- self.root.join("../rg")
|
||||||
|
- } else {
|
||||||
|
- path
|
||||||
|
- }
|
||||||
|
- }
|
||||||
|
-
|
||||||
|
- /// Returns the path to the ripgrep executable.
|
||||||
|
- #[cfg(windows)]
|
||||||
|
pub fn bin(&self) -> PathBuf {
|
||||||
|
- let path = self.root.join("rg.exe");
|
||||||
|
- if !path.is_file() {
|
||||||
|
- // Looks like a recent version of Cargo changed the cwd or the
|
||||||
|
- // location of the test executable.
|
||||||
|
+ if cfg!(windows) {
|
||||||
|
self.root.join("../rg.exe")
|
||||||
|
} else {
|
||||||
|
- path
|
||||||
|
+ self.root.join("../rg")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
--
|
||||||
|
2.18.0
|
||||||
|
|
||||||
Loading…
Reference in new issue