r211625 - Disable the bits of r211623 that broke the bots

Eric Christopher echristo at gmail.com
Tue Jun 24 14:18:50 PDT 2014


On Tue, Jun 24, 2014 at 2:18 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>
>> On Jun 24, 2014, at 2:00 PM, Eric Christopher <echristo at gmail.com> wrote:
>>
>> On Tue, Jun 24, 2014 at 1:57 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>>>
>>>> On Jun 24, 2014, at 1:51 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>>
>>>> I missed that this is in a testcase and not in the main code. I'm
>>>> definitely less worked up about it, but perhaps some more detail than
>>>> just the FIXME would be nice :)
>>>
>>> Hey Eric,
>>>
>>> Fair enough, the comment sucks and you’re right this isn’t a great way to deal with test failures in general. FWIW, I’m working on this right now and I didn’t want to revert entirely, because I was hoping to get some reassurance from the bots that the rest of the patch was ok.  It’s hard to test filesystem-ish changes because of the potential for platform differences :-)
>>>
>>
>> Yeah, you're absolutely right. I'd misread it and thought it was the
>> main code, but a testcase you just added that's parsing a bit weird on
>> a system you don't have I'm much less concerned about. Great that
>> you're looking at it right now, sometimes it's not clear if it's a
>> "now" versus "soon" thing. :)
>
> Should be fixed in r211633 (also fixed another test bug from this commit hitting a few bots).
>

Awesome, thanks Ben, sorry for the noise.

-eric

> Ben
>
>>
>> Thanks!
>>
>> -eric
>>
>>> Cheers,
>>>
>>> Ben
>>>
>>>>
>>>> Thanks!
>>>>
>>>> -eric
>>>>
>>>> On Tue, Jun 24, 2014 at 1:39 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>>> Please don't do this in this way, just go ahead and revert the whole
>>>>> patch and figure it out.
>>>>>
>>>>> -eric
>>>>>
>>>>> On Tue, Jun 24, 2014 at 1:00 PM, Ben Langmuir <blangmuir at apple.com> wrote:
>>>>>> Author: benlangmuir
>>>>>> Date: Tue Jun 24 15:00:30 2014
>>>>>> New Revision: 211625
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=211625&view=rev
>>>>>> Log:
>>>>>> Disable the bits of r211623 that broke the bots
>>>>>>
>>>>>> Part of my test seems to rely on iterator bits that I didn't implement,
>>>>>> at least in the gcc bots. Disabling while I investigate.
>>>>>>
>>>>>> Modified:
>>>>>>   cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
>>>>>>
>>>>>> Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
>>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=211625&r1=211624&r2=211625&view=diff
>>>>>> ==============================================================================
>>>>>> --- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
>>>>>> +++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Tue Jun 24 15:00:30 2014
>>>>>> @@ -395,22 +395,23 @@ TEST(VirtualFileSystemTest, HiddenInIter
>>>>>>    checkContents(O->dir_begin("/", EC), Contents);
>>>>>>  }
>>>>>>
>>>>>> +  // FIXME: broke gcc build
>>>>>>  // Make sure we get the top-most entry
>>>>>> -  vfs::directory_iterator E;
>>>>>> -  {
>>>>>> -    auto I = std::find_if(O->dir_begin("/", EC), E, [](vfs::Status S){
>>>>>> -      return S.getName() == "/hiddenByUp";
>>>>>> -    });
>>>>>> -    ASSERT_NE(E, I);
>>>>>> -    EXPECT_EQ(sys::fs::owner_all, I->getPermissions());
>>>>>> -  }
>>>>>> -  {
>>>>>> -    auto I = std::find_if(O->dir_begin("/", EC), E, [](vfs::Status S){
>>>>>> -      return S.getName() == "/hiddenByMid";
>>>>>> -    });
>>>>>> -    ASSERT_NE(E, I);
>>>>>> -    EXPECT_EQ(sys::fs::owner_write, I->getPermissions());
>>>>>> -  }
>>>>>> +  // vfs::directory_iterator E;
>>>>>> +  // {
>>>>>> +  //   auto I = std::find_if(O->dir_begin("/", EC), E, [](vfs::Status S){
>>>>>> +  //     return S.getName() == "/hiddenByUp";
>>>>>> +  //   });
>>>>>> +  //   ASSERT_NE(E, I);
>>>>>> +  //   EXPECT_EQ(sys::fs::owner_all, I->getPermissions());
>>>>>> +  // }
>>>>>> +  // {
>>>>>> +  //   auto I = std::find_if(O->dir_begin("/", EC), E, [](vfs::Status S){
>>>>>> +  //     return S.getName() == "/hiddenByMid";
>>>>>> +  //   });
>>>>>> +  //   ASSERT_NE(E, I);
>>>>>> +  //   EXPECT_EQ(sys::fs::owner_write, I->getPermissions());
>>>>>> +  // }
>>>>>> }
>>>>>>
>>>>>> // NOTE: in the tests below, we use '//root/' as our root directory, since it is
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> cfe-commits mailing list
>>>>>> cfe-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>




More information about the cfe-commits mailing list