[lld] r307459 - Delete the pdb diff test.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 10 10:37:47 PDT 2017
The test isn't testing the functionality that was committed. The
functionality that was committed was committed in order to test existing
functionality that has never had test coverage to begin with. So I think
it comes down to "where do you draw the line". I could commit tests for
the functionality that is there specifically to test other functionality,
and would I then need to commit tests for that too? :) In any case, I
should have a fix in the next couple hours.
On Mon, Jul 10, 2017 at 10:21 AM David Blaikie <dblaikie at gmail.com> wrote:
> Sure - but functionality without test coverage isn't OK (in the same way
> the patch wouldn't be acceptable if it didn't have a test to start with,
> committing it then removing the test coverage doesn't seem OK to me)
>
> On Mon, Jul 10, 2017 at 10:20 AM Zachary Turner <zturner at google.com>
> wrote:
>
>> The functionality actually works, it's just a problem with the way the
>> test is written which is not entirely easy to fix.
>>
>> On Mon, Jul 10, 2017 at 10:18 AM David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>> Generally I would expect a whole patch should probably be reverted if
>>> the tests are failing - rather than undoing/removing the tests but leaving
>>> the functionality in-tree.
>>>
>>> On Fri, Jul 7, 2017 at 7:54 PM Zachary Turner via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: zturner
>>>> Date: Fri Jul 7 19:54:19 2017
>>>> New Revision: 307459
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=307459&view=rev
>>>> Log:
>>>> Delete the pdb diff test.
>>>>
>>>> This is failing on Linux for unknown reasons, so I need to
>>>> get the bots green while I investigate.
>>>>
>>>> Removed:
>>>> lld/trunk/test/COFF/pdb-diff.test
>>>>
>>>> Removed: lld/trunk/test/COFF/pdb-diff.test
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/pdb-diff.test?rev=307458&view=auto
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/test/COFF/pdb-diff.test (original)
>>>> +++ lld/trunk/test/COFF/pdb-diff.test (removed)
>>>> @@ -1,212 +0,0 @@
>>>> -This test verifies that we produce PDBs compatible with MSVC in
>>>> various ways.
>>>> -We check in a cl-generated object file, PDB, and original source which
>>>> serve
>>>> -as the "baseline" for us to measure against. Then we link the same
>>>> object
>>>> -file with LLD and compare the two PDBs. Since the baseline object
>>>> file and
>>>> -PDB are already checked in, we just run LLD on the object file.
>>>> -
>>>> -RUN: lld-link /debug /pdb:%T/pdb-diff-lld.pdb /nodefaultlib
>>>> /entry:main %S/Inputs/pdb-diff.obj
>>>> -RUN: llvm-pdbutil diff -result -values=false %T/pdb-diff-lld.pdb
>>>> %S/Inputs/pdb-diff-cl.pdb | FileCheck %s
>>>> -
>>>> -CHECK: ----------------------
>>>> -CHECK-NEXT: | MSF Super Block |
>>>> -CHECK-NEXT: |----------------+---|
>>>> -CHECK-NEXT: | File | |
>>>> -CHECK-NEXT: |----------------+---|
>>>> -CHECK-NEXT: | Block Size | I |
>>>> -CHECK-NEXT: |----------------+---|
>>>> -CHECK-NEXT: | Block Count |
>>>> -CHECK-NEXT: |----------------+---|
>>>> -CHECK-NEXT: | Unknown 1 | I |
>>>> -CHECK-NEXT: |----------------+---|
>>>> -CHECK-NEXT: | Directory Size |
>>>> -CHECK-NEXT: |----------------+---|
>>>> -CHECK-NEXT: ------------------------------------
>>>> -CHECK-NEXT: | Stream Directory |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | File | |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Stream Count | D |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Old MSF Directory | I |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | PDB Stream | I |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | TPI Stream | I |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | DBI Stream | I |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | IPI Stream | I |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | New FPO Data | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Section Header Data | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Named Stream "/names" | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Named Stream "/LinkInfo" | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Module "{{.*}}\pdb-diff.obj" | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Module "* Linker *" | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | TPI Hash | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | IPI Hash | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Global Symbol Hash | D |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Public Symbol Hash | D |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Public Symbol Records | D |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: ------------------------------------
>>>> -CHECK-NEXT: | String Table |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | File | |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Number of Strings | D |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Hash Version | I |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Byte Size |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Signature | I |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | Empty Strings |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | {{.*}}pdb-diff.cpp | {{[EI]}} |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | $T0 $ebp = $...p $T0 8 + = | D |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: | d:\src\llvm-...er internal) | D |
>>>> -CHECK-NEXT: |------------------------------+---|
>>>> -CHECK-NEXT: ----------------------------
>>>> -CHECK-NEXT: | PDB Stream |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | File | |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Stream Size |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Age | I |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Guid | D |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Signature | D |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Version | I |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Features (set) | I |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Feature | I |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Named Stream Size |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | Named Streams (map) | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | /names | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: | /LinkInfo | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------+---|
>>>> -CHECK-NEXT: ----------------------------------------------
>>>> -CHECK-NEXT: | DBI Stream |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | File | |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Dbi Version | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Age | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Machine | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Flags | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Build Major | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Build Minor | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Build Number | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | PDB DLL Version | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | PDB DLL RBLD | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (FPO) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (Exception) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (Fixup) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (OmapToSrc) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (OmapFromSrc) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (SectionHdr) | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (TokenRidMap) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (Xdata) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (Pdata) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (NewFPO) | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | DBG (SectionHdrOrig) | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Globals Stream | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Publics Stream | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Symbol Records | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Has CTypes | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Is Incrementally Linked | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Is Stripped | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Module Count | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Source File Count | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: |Module "{{.*}}\pdb-diff.obj"|
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Modi | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Obj File Name | {{[DEI]}} |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Debug Stream | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - C11 Byte Size | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - C13 Byte Size | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - # of files | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Pdb File Path Index | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Source File Name Index | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Symbol Byte Size | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | Module "* Linker *" |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Modi | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Obj File Name | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Debug Stream | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - C11 Byte Size | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - C13 Byte Size | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - # of files | I |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Pdb File Path Index | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Source File Name Index | {{[EI]}} |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -CHECK-NEXT: | - Symbol Byte Size | D |
>>>> -CHECK-NEXT: |----------------------------------------+---|
>>>> -
>>>> -
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170710/28f167ea/attachment.html>
More information about the llvm-commits
mailing list