[PATCH] D83834: Add test utility 'extract'

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/test/tools/extract/basic.s:2
+# REQUIRES: x86-registered-target
+# RUN: extract aa %s | llvm-mc -triple=x86_64 - 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=AA --implicit-check-not='warning: bb'
----------------
MaskRay wrote:
> grimar wrote:
> > grimar wrote:
> > > MaskRay wrote:
> > > > jhenderson wrote:
> > > > > Do you actually need to use llvm-mc at all? It seems a bit heavy duty. You could just use `FileCheck` directly on the output of `extract`, e.g.
> > > > > 
> > > > > ```
> > > > > # RUN: extract aa %s | FileCheck %s --check-prefix=AA --implicit-check-not=bb
> > > > > ...
> > > > > 
> > > > > # AA: {{^}}aa{{$}}
> > > > > 
> > > > > #--- aa
> > > > > aa
> > > > > ```
> > > > > 
> > > > > I suppose that doesn't cover the line numbering, but perhaps that should be a different test?
> > > > I suppose the only problem is `# REQUIRES: x86-registered-target` but this is the simplest tool (I can find) which can print line number information, and we do need to test several forms and ensure each form can get correct line numbers.
> > > > 
> > > > If you have suggestion for another tool without a need to add `# REQUIRES: x86-registered-target`, I'd happily change. But I'd prefer sticking with llvm-mc otherwise.
> > > Perhaps, you could use the LLVM`s `count` tool to count the number of lines?
> > Another way I can think of is to add more sample files to compare with.
> > I.e. you can create sample.a, sample.b etc. And then compare the output of extract with those samples.
> > Will this work? I just also do not think that using of `llvm-mc` is a good idea for the tool that splits the text lines...
> > 
> > Perhaps, you could use the LLVM`s count tool to count the number of lines?
> 
> This seems like an indirect way to test something.
> 
> > I.e. you can create sample.a, sample.b etc. And then compare the output of extract with those samples.
> 
> This degrades readability. Adding a line means all Inputs/ files need updating.
> 
> .warning has well defined and very stable interface. We use nothing but a very basic feature set of lexing of it. No assembly, no disassembly. I don't see a problem using it for testing.
> > I.e. you can create sample.a, sample.b etc. And then compare the output of extract with those samples.
> This degrades readability. Adding a line means all Inputs/ files need updating.

I think this (having separate test files, diffing them exactly) is the right way to test this tool - the nature of low-level tools that are designed to improve the readability of higher level tests is that testing them will be less elegant. (eg: testing yaml2obj if we intend to use it to test llvm-dwarfdump means yaml2obj tests can't use llvm-dwarfdump to test them, etc)

Indeed, presumably we want to use this tool when testing llvm-mc, so we should not use llvm-mc to test the tool - otherwise the testing is circular.

Please don't use llvm-mc or similar things to test this very low-level piece of test infrastructure like this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83834/new/

https://reviews.llvm.org/D83834