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

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

On Mon, Dec 10, 2018 at 1:18 PM Stella Stamenova <stilis at microsoft.com> wrote:
>
> The failure that you are getting when you run on the d:\ drive is what we were seeing before this change in our testing. What do you get without this change?

When I reverted the change, I got the same behavior -- so then I
double-checked my build logs and discovered the issue. For some
reason, the unit tests were not rebuilding for me despite having dirty
files. When I un-reverted the changes and manually rebuilt
BasicTests.exe, the test started passing. So I think this was a local
build issue where my implementation got into a confused state. Sorry
for the noise!

~Aaron

>
> Thanks,
> -Stella
>
> -----Original Message-----
> From: Aaron Ballman <aaron at aaronballman.com>
> Sent: Monday, December 10, 2018 10:11 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 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%7C72f
> > > 98
> > > 8bf86f141af91ab2d7cd011db47%7C1%7C0%7C636800505193880574&sdata=v
> > > v0
> > > 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%2Fre
> > > vi
> > > ews.llvm.org%2FD55451&data=02%7C01%7Cstilis%40microsoft.com%7Cc4
> > > e9
> > > 0b6970994d2b15b608d65eaf7ff3%7C72f988bf86f141af91ab2d7cd011db47%7C1%
> > > 7C
> > > 0%7C636800505193880574&sdata=0dFS4XR2o85FyI0XHr9Eh4pX%2BbdC2CPhX
> > > dd
> > > 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%2FFile
> > > Ma
> > > nagerTest.cpp%3Frev%3D348665%26r1%3D348664%26r2%3D348665%26view%3Ddi
> > > ff
> > > &data=02%7C01%7Cstilis%40microsoft.com%7Cc4e90b6970994d2b15b608d
> > > 65
> > > eaf7ff3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636800505193880
> > > 57
> > > 4&sdata=UkdKXFFYeXogiCIJ8%2B41qUFO2qON9seRUVYziL2%2B9Yg%3D&r
> > > es
> > > 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%2Flis
> > > ts.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits&data=02
> > > %7C01%7Cstilis%40microsoft.com%7C2bf97a4de6254e96167008d65ecad891%7C
> > > 72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636800622648102157&sd
> > > ata=aVXLXRHK1Kp8LLjl1QQPH6pBXjg1GUflb2%2FqTUjNGgU%3D&reserved=0

More information about the cfe-commits mailing list