On Fri, Jun 08, 2018 at 09:34:02AM -0600, Ross Zwisler wrote: > On Fri, Jun 08, 2018 at 07:17:51AM +0200, Thomas Huth wrote: > > On 08.06.2018 01:09, Michael S. Tsirkin wrote: > > > On Thu, Jun 07, 2018 at 04:31:08PM -0600, Ross Zwisler wrote: > > >> Currently if "make check" detects a mismatch in the ASL generated during > > >> testing, we print an error such as: > > >> > > >> acpi-test: Warning! SSDT mismatch. Actual [asl:/tmp/asl-QZDWJZ.dsl, > > >> aml:/tmp/aml-T8JYJZ], Expected [asl:/tmp/asl-DTWVJZ.dsl, > > >> aml:tests/acpi-test-data/q35/SSDT.dimmpxm]. > > >> > > >> but the testing still exits with good shell status. This is wrong, and > > >> makes bisecting such a failure difficult. > > >> > > >> Signed-off-by: Ross Zwisler <ross.zwis...@linux.intel.com> > > > > > > Failing would also mean that any change must update the expected files > > > at the same time. And that in turn is problematic because expected > > > files are binary and can't be merged. > > > > > > In other words the way we devel ACPI right now means that bisect will > > > periodically produce a diff, it's not an error. > > > > But apparently the current way also allows that real bug go unnoticed > > for a while, until somebody accidentially spots the warning in the > > output of "make check". Wouldn't it be better to fail at CI time > > already? If a merge of the file is required, you can still resolve that > > manually (i.e. by rebasing one of the pull requests). > > I share this point of view. The unit tests only add value if we keep them up > to date and passing as we modify the source. The ACPI tables in this case > were broken in an innocuous way and just needed to be updated to match again, > but it means that the tests for them are now basically turned off.
The expected value tests are a debugging aid. They do not catch bugs and aren't designed to. In particular the comparisons do not even run if IASL isn't installed. > Someone > else could come along and break the ACPI table in a real and harmful way, and > nobody would notice because the and result would still just be an ACPI table > mismatch that is non-fatal and ignored. There are tests with windows and linux guests that will catch it. It's just not something we can handle reasonably in a unit test. -- MST