On Mon, Dec 10, 2018 at 1:18 PM Stella Stamenova <sti...@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 <aa...@aaronballman.com> > Sent: Monday, December 10, 2018 10:11 AM > To: Stella Stamenova <sti...@microsoft.com> > Cc: cfe-commits <cfe-commits@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 <sti...@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 <aa...@aaronballman.com> > > Sent: Monday, December 10, 2018 6:55 AM > > To: Stella Stamenova <sti...@microsoft.com> > > Cc: cfe-commits <cfe-commits@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@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@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits