r348665 - [tests] Fix the FileManagerTest getVirtualFile test on Windows

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 10 10:10:49 PST 2018


On Mon, Dec 10, 2018 at 12:30 PM Stella Stamenova <stilis at microsoft.com> wrote:
>
> Our tests run on drive E:\ (not C:\) which is why we saw this test failing. After this change, the test can now run successfully for us because the temporary files are created and checked for on the C:\ drive. Before this change, the temporary files were created on the E:\ drive and checked for on the C:\ drive.
>
> Are you saying that you have a drive C:\ and the test is failing anyway? Or do you not even have a drive C:\?

I have two drives, C:\ and D:\. When I run this unit test (from my D
drive), I now get:

[ RUN      ] FileManagerTest.getVirtualFileFillsRealPathName
D:\llvm\tools\clang\unittests\Basic\FileManagerTest.cpp(371): error:
    Expected: file->tryGetRealPathName()
      Which is: { 'd' (100, 0x64), ':' (58, 0x3A), '/' (47, 0x2F), 't'
(116, 0x74), 'm' (109, 0x6D), 'p' (112, 0x70), '\\' (92, 0x5C), 't'
(116, 0x74), 'e' (101, 0x65), 's' (115, 0x73), 't' (116, 0x74) }
To be equal to: ExpectedResult
      Which is: { 'C' (67, 0x43), ':' (58, 0x3A), '/' (47, 0x2F), 't'
(116, 0x74), 'm' (109, 0x6D), 'p' (112, 0x70), '\\' (92, 0x5C), 't'
(116, 0x74), 'e' (101, 0x65), 's' (115, 0x73), 't' (116, 0x74) }
[  FAILED  ] FileManagerTest.getVirtualFileFillsRealPathName (2 ms)

I do not have a C:\temp or a D:\temp drive. If I move the test
executable onto my C:\ drive, I get a different failure instead:

[ RUN      ] FileManagerTest.getVirtualFileFillsRealPathName
D:\llvm\tools\clang\unittests\Basic\FileManagerTest.cpp(371): error:
    Expected: file->tryGetRealPathName()
      Which is: { 'c' (99, 0x63), ':' (58, 0x3A), '/' (47, 0x2F), 't'
(116, 0x74), 'm' (109, 0x6D), 'p' (112, 0x70), '\\' (92, 0x5C), 't'
(116, 0x74), 'e' (101, 0x65), 's' (115, 0x73), 't' (116, 0x74) }
To be equal to: ExpectedResult
      Which is: { 'C' (67, 0x43), ':' (58, 0x3A), '/' (47, 0x2F), 't'
(116, 0x74), 'm' (109, 0x6D), 'p' (112, 0x70), '\\' (92, 0x5C), 't'
(116, 0x74), 'e' (101, 0x65), 's' (115, 0x73), 't' (116, 0x74) }
[  FAILED  ] FileManagerTest.getVirtualFileFillsRealPathName (2 ms)

~Aaron

>
> Thanks,
> -Stella
>
> -----Original Message-----
> From: Aaron Ballman <aaron at aaronballman.com>
> Sent: Monday, December 10, 2018 6:55 AM
> To: Stella Stamenova <stilis at microsoft.com>
> Cc: cfe-commits <cfe-commits at lists.llvm.org>
> Subject: Re: r348665 - [tests] Fix the FileManagerTest getVirtualFile test on Windows
>
> On Fri, Dec 7, 2018 at 6:52 PM Stella Stamenova via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> >
> > Author: stella.stamenova
> > Date: Fri Dec  7 15:50:05 2018
> > New Revision: 348665
> >
> > URL:
> > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.
> > org%2Fviewvc%2Fllvm-project%3Frev%3D348665%26view%3Drev&data=02%7C
> > 01%7Cstilis%40microsoft.com%7Cc4e90b6970994d2b15b608d65eaf7ff3%7C72f98
> > 8bf86f141af91ab2d7cd011db47%7C1%7C0%7C636800505193880574&sdata=vv0
> > qe64wy0oqci7RAlCXHxmjREevqS3s%2ByNz83tqQIw%3D&reserved=0
> > Log:
> > [tests] Fix the FileManagerTest getVirtualFile test on Windows
> >
> > Summary: The test passes on Windows only when it is executed on the C: drive. If the build and tests run on a different drive, the test is currently failing.
> >
> > Reviewers: kadircet, asmith
> >
> > Subscribers: cfe-commits
> >
> > Differential Revision:
> > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Frevi
> > ews.llvm.org%2FD55451&data=02%7C01%7Cstilis%40microsoft.com%7Cc4e9
> > 0b6970994d2b15b608d65eaf7ff3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C
> > 0%7C636800505193880574&sdata=0dFS4XR2o85FyI0XHr9Eh4pX%2BbdC2CPhXdd
> > X3lDYaao%3D&reserved=0
> >
> > Modified:
> >     cfe/trunk/unittests/Basic/FileManagerTest.cpp
> >
> > Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
> > URL:
> > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.
> > org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Funittests%2FBasic%2FFileMa
> > nagerTest.cpp%3Frev%3D348665%26r1%3D348664%26r2%3D348665%26view%3Ddiff
> > &data=02%7C01%7Cstilis%40microsoft.com%7Cc4e90b6970994d2b15b608d65
> > eaf7ff3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63680050519388057
> > 4&sdata=UkdKXFFYeXogiCIJ8%2B41qUFO2qON9seRUVYziL2%2B9Yg%3D&res
> > erved=0
> > ======================================================================
> > ========
> > --- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
> > +++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Fri Dec  7 15:50:05
> > +++ 2018
> > @@ -351,22 +351,34 @@ TEST_F(FileManagerTest, makeAbsoluteUses
> >
> >  // getVirtualFile should always fill the real path.
> >  TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
> > +  SmallString<64> CustomWorkingDir;
> > +#ifdef _WIN32
> > +  CustomWorkingDir = "C:/";
> > +#else
> > +  CustomWorkingDir = "/";
> > +#endif
>
> Unfortunately, this is still an issue -- you cannot assume that C:\ is the correct drive letter on Windows. For instance, this unit test fails for me because it turns out to be D:\ on my system.
>
> ~Aaron
>
> > +
> > +  auto FS = IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>(
> > +      new llvm::vfs::InMemoryFileSystem);  //
> > + setCurrentworkingdirectory must finish without error.
> > +  ASSERT_TRUE(!FS->setCurrentWorkingDirectory(CustomWorkingDir));
> > +
> > +  FileSystemOptions Opts;
> > +  FileManager Manager(Opts, FS);
> > +
> >    // Inject fake files into the file system.
> >    auto statCache = llvm::make_unique<FakeStatCache>();
> >    statCache->InjectDirectory("/tmp", 42);
> >    statCache->InjectFile("/tmp/test", 43);
> > -  manager.addStatCache(std::move(statCache));
> > +
> > +  Manager.addStatCache(std::move(statCache));
> >
> >    // Check for real path.
> > -  const FileEntry *file = manager.getVirtualFile("/tmp/test", 123,
> > 1);
> > +  const FileEntry *file = Manager.getVirtualFile("/tmp/test", 123,
> > + 1);
> >    ASSERT_TRUE(file != nullptr);
> >    ASSERT_TRUE(file->isValid());
> > -  SmallString<64> ExpectedResult;
> > -#ifdef _WIN32
> > -  ExpectedResult = "C:/";
> > -#else
> > -  ExpectedResult = "/";
> > -#endif
> > +  SmallString<64> ExpectedResult = CustomWorkingDir;
> > +
> >    llvm::sys::path::append(ExpectedResult, "tmp", "test");
> >    EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);  }
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits&data=02%7C01%7Cstilis%40microsoft.com%7Cc4e90b6970994d2b15b608d65eaf7ff3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636800505193880574&sdata=LRq7kLE%2BrPavTkQ2gSnc%2B%2FHrPMc8V1QQLWKr2gIPSBs%3D&reserved=0


More information about the cfe-commits mailing list