[Lldb-commits] [PATCH] D66370: [dotest] Add a dotest flag `--force-dwarf` to override the tested DWARF version.

Mon Aug 19 06:22:52 PDT 2019

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This looks fine to me. We spoke about this on irc, but I'll just repeat what I said here for the record. Running the dotest tests with different compilers/architectures/dwarf versions/etc. is very useful for increasing coverage, finding new bugs, etc. However, I don't think we should be using this facility as a first or only line of defence against regressions. Specifically, I'd like to avoid arguments like "there's no need to add tests for this because this code is exercised when running the test suite with gcc-47 and dwarf-13 on a Tuesday wearing shorts". New features/bugfixes should be testable in the default configuration, and this facility should be the "extra mile".

================
Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:353
+
+    os.environ['CFLAGS_EXTRAS'] = cflags_extras

----------------
aprantl wrote:
> Does this actually work with tests whose Makefiles manually set CFLAGS_EXTRAS? I.e., do we need to distinguish between extra CFLAGS that come in from the layer above and ones that are meant to be customized in the individual test Makefiles?
If makefiles do `CFLAGS_EXTRAS += ...` instead of `CFLAGS_EXTRAS = foo` (which I think most do), then everything should be fine.

================
Comment at: lldb/packages/Python/lldbsuite/test/dotest_args.py:129
+    group.add_argument(
+        '--force-dwarf',
+        metavar='dwarf_version',
----------------
Maybe just call the argument `--dwarf-version`, just like the source variable?

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66370