Attention is currently required from: Mark Mentovai.
1 comment:
Patchset:
PTAL - simple fix to a couple of incorrect test conditions.
To view, visit change 4348801. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Bruce Dawson.
1 comment:
File util/file/filesystem_test.cc:
Patch Set #2, Line 480: GTEST_SKIP();
I don't think we need to skip the whole test. We can just skip the parts that involve symbolic links. Follow the `!IS_FUCHSIA` lead below: Fuchsia doesn't have symbolic links either. Applies to both tests.
To view, visit change 4348801. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Mark Mentovai.
1 comment:
File util/file/filesystem_test.cc:
Patch Set #2, Line 480: GTEST_SKIP();
I don't think we need to skip the whole test. […]
Good point. Done.
To view, visit change 4348801. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Bruce Dawson.
1 comment:
File util/file/filesystem_test.cc:
Patch Set #3, Line 538: EXPECT_EQ(filesize, 2 * sizeof(kTestFileContent));
We really do want to reach this, it's the whole point of the test, and it doesn't depend on symbolic links. So no `GTEST_SKIP` in this one, just make the stuff that's preprocessored out on Fuchsia be skipped at runtime when symbolic links aren't supported.
To view, visit change 4348801. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Mark Mentovai.
1 comment:
File util/file/filesystem_test.cc:
Patch Set #3, Line 538: EXPECT_EQ(filesize, 2 * sizeof(kTestFileContent));
We really do want to reach this, it's the whole point of the test, and it doesn't depend on symbolic […]
Gotcha. I didn't notice the code at the end after the #ifdef. Fixed.
To view, visit change 4348801. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Bruce Dawson.
Patch set 4:Code-Review +1
1 comment:
Patchset:
Thanks!
To view, visit change 4348801. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Bruce Dawson.
Patch set 4:Commit-Queue +2
Crashpad LUCI CQ submitted this change.
Skip tests that create symbol links when not allowed
Several tests in filesystem_test.cc create symbol links. The privilege
needed to do this is not enabled on all Windows systems so several of
the tests check for the privilege and are skipped if it is not
available.
However, two tests that created symbol links were not doing this check
and therefore failed on some Windows machines. This corrects those
failures by adding the checks.
Bug: chromium:1418165
Change-Id: I6621796b462b8db02271ad5a05e0c29ee047f648
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4348801
Reviewed-by: Mark Mentovai <ma...@chromium.org>
Commit-Queue: Bruce Dawson <bruce...@chromium.org>
---
M util/file/filesystem_test.cc
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/util/file/filesystem_test.cc b/util/file/filesystem_test.cc
index eb492cd..bff3e32 100644
--- a/util/file/filesystem_test.cc
+++ b/util/file/filesystem_test.cc
@@ -487,6 +487,10 @@
EXPECT_EQ(filesize, sizeof(kTestFileContent));
#if !BUILDFLAG(IS_FUCHSIA)
+ if (!CanCreateSymbolicLinks()) {
+ GTEST_SKIP();
+ }
+
// Create a link to a file.
base::FilePath link(temp_dir.path().Append(FILE_PATH_LITERAL("link")));
ASSERT_TRUE(CreateSymbolicLink(filepath, link));
@@ -517,13 +521,15 @@
writer2.Close();
#if !BUILDFLAG(IS_FUCHSIA)
- // Create a link to a file.
- base::FilePath link(dir.Append(FILE_PATH_LITERAL("link")));
- ASSERT_TRUE(CreateSymbolicLink(filepath2, link));
+ if (CanCreateSymbolicLinks()) {
+ // Create a link to a file.
+ base::FilePath link(dir.Append(FILE_PATH_LITERAL("link")));
+ ASSERT_TRUE(CreateSymbolicLink(filepath2, link));
- // Create a link to a dir.
- base::FilePath linkdir(temp_dir.path().Append(FILE_PATH_LITERAL("link")));
- ASSERT_TRUE(CreateSymbolicLink(dir, linkdir));
+ // Create a link to a dir.
+ base::FilePath linkdir(temp_dir.path().Append(FILE_PATH_LITERAL("link")));
+ ASSERT_TRUE(CreateSymbolicLink(dir, linkdir));
+ }
#endif // !BUILDFLAG(IS_FUCHSIA)
uint64_t filesize = GetDirectorySize(temp_dir.path());
To view, visit change 4348801. To unsubscribe, or for help writing mail filters, visit settings.