From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37141) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF4QA-0006K5-Al for qemu-devel@nongnu.org; Wed, 06 Aug 2014 12:50:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XF4Q4-00067G-0H for qemu-devel@nongnu.org; Wed, 06 Aug 2014 12:50:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9102) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF4Q3-00067C-Kz for qemu-devel@nongnu.org; Wed, 06 Aug 2014 12:50:31 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s76GoUSp010007 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 6 Aug 2014 12:50:31 -0400 Message-ID: <53E25CCF.9090000@redhat.com> Date: Wed, 06 Aug 2014 12:50:23 -0400 From: John Snow MIME-Version: 1.0 References: <1407186691-16103-1-git-send-email-jsnow@redhat.com> <20140806094327.GB17426@stefanha-thinkpad.redhat.com> <87egwtsvmr.fsf@blackfin.pond.sub.org> In-Reply-To: <87egwtsvmr.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 00/30] AHCI test suite framework List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , Stefan Hajnoczi Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, mst@redhat.com On 08/06/2014 07:30 AM, Markus Armbruster wrote: > Stefan Hajnoczi writes: > >> On Mon, Aug 04, 2014 at 05:11:01PM -0400, John Snow wrote: >>> This patch series introduces a number of small fixes and tweaks to >>> help support an AHCI test suite that in the future I hope to expand >>> to a fuller regression suite to help guide the development of the >>> AHCI device support under, in particular, the Q35 machine type in QEMU. >>> >>> Paolo Bonzini has contributed a number of cleanup and refactoring patches >>> that support changes to the PIO setup FIS packet construction code, which >>> is necessary for testing ths specification adherence of the IDENTIFY command, >>> which issues its data exclusively via PIO mechanisms. >>> >>> The ahci-test code being checked in represents a minimum of functionality >>> needed in order to issue and receive commands from the AHCI HBA under the >>> libqos / qtest environment. >>> >>> In V2, as detailed below, these tests are not currently expected to pass. >>> I will post a complementary patch outside of this set that highlights >>> the exact set of tests that will not pass, which can help verify at least >>> the portions of these tests that do work correctly. >>> >>> Assertions that currently fail: >>> - Ordering of PCI capabilities as defined by either AHCI or Intel ICH9 >>> - Boot-time values of the PxTFD register, which should not have valid >>> data until after a D2H FIS is received, but does in Qemu 2.1 >>> - Boot-time values of the PxSIG register, which should have a specific >>> placeholder signature until the first D2H FIS is received, but is >>> currently blank. >>> - The "Descriptor Processed" interrupt is expected after the IDENTIFY >>> command exhausts the given PRDT, but is not seen. >> >> I guess these are the assertion failures: >> ERROR:tests/ahci-test.c:777:ahci_test_pci_spec: assertion failed >> ((data & 0xFF) == PCI_CAP_ID_MSI): (0x00000012 == 0x00000005) >> GTester: last random seed: R02Sd92815a5d013e8433808b903b2b13fb0 >> ** >> ERROR:tests/ahci-test.c:1165:ahci_test_port_spec: assertion failed >> ((reg) & ((0x01)) == ((0x01))): (0x00000000 == 0x00000001) >> GTester: last random seed: R02S4d6c05e864dc777e64141cdc6d2a18cf >> ** >> ERROR:tests/ahci-test.c:1360:ahci_test_identify: assertion failed >> ((reg) & ((0x20)) == ((0x20))): (0x00000000 == 0x00000020) >> GTester: last random seed: R02S2b3b330b83a66badb24da80b48120b1d >> >> Why publish this patch series if the test fails? We can't merge failing >> tests. > > Correct. > > What I do when I want to start some bug fixing work with tests is to > write the tests to expect the actual, incorrect behavior, with a > greppable comment documenting the correct behavior. Then clean that up > as the bugs get fixed. > I thought it was valid to submit a failing test if... well, the behavior was wrong. Stefan said no warnings, so I took that to mean "This should fail." I didn't think it was too strange to have a failing test for something that was not feature complete. So, if it's not appropriate to have a failing test at any stage (Regressions only?) now's a good time to let me know how you would like me to accomplish no warnings but have the tests pass. In my V1 I did just print a "WARN" string which was reasonable greppable to find the failure cases. My next guess at something workable would be to stick the assertions behind a bool that could be toggled on/off via a flag that could be toggled with --all or similar to hit the expected failure cases. No warnings inside of the test harness, no failures, and cases could be found by grepping the name of the boolean and/or some accompanying comment. --j