Re: [SeaBIOS] [Qemu-devel] [QEMU RFC PATCH 6/7] i386: topology & APIC ID utility functions
On Mon, Jul 23, 2012 at 07:44:44PM +0000, Blue Swirl wrote:
On Mon, Jul 23, 2012 at 7:28 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Jul 23, 2012 at 07:11:11PM +0000, Blue Swirl wrote:
On Mon, Jul 23, 2012 at 6:59 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Jul 23, 2012 at 04:49:07PM +0000, Blue Swirl wrote:
On Mon, Jul 16, 2012 at 5:42 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Sat, Jul 14, 2012 at 09:14:30AM +0000, Blue Swirl wrote: [...] > >> > diff --git a/tests/Makefile b/tests/Makefile > >> > index b605e14..89bd890 100644 > >> > --- a/tests/Makefile > >> > +++ b/tests/Makefile > >> > @@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF) > >> > check-unit-y += tests/test-coroutine$(EXESUF) > >> > check-unit-y += tests/test-visitor-serialization$(EXESUF) > >> > check-unit-y += tests/test-iov$(EXESUF) > >> > +check-unit-y += tests/test-x86-cpuid$(EXESUF) > >> > >> This probably tries to build the cpuid test also for non-x86 targets > >> and break them all. > > > > I don't think there's any concept of "targets" for the check-unit tests. > > How about: > check-qtest-i386-y = tests/test-x86-cpuid$(EXESUF)
test-x86-cpuid is not a qtest test case.
Why not? I don't think it is a unit test either, judging from what the other unit tests do.
It is absolutely a unit test. I don't know why you don't think so. It simply checks the results of the APIC ID calculation functions.
Yes, but the other 'unit tests' (the names used here are very confusing, btw) check supporting functions like coroutines, not hardware. Hardware tests (qtest) need to bind to an architecture, in this case x86.
test-x86-cpuid doesn't check hardware, either. It just checks the simple functions that calculate APIC IDs (to make sure the math is correct). Those functions aren't even used by any hardware emulation code until later in the patch series (when the CPU initialization code gets changed to use the new function).
By that time the function is clearly x86 HW and the check is an x86 hardware check. QEMU as whole consists of simple functions that calculate something.
It's useful onily for a specific architecture, yes, but it surely doesn't make libqtest necessary. That's the difference between the unit tests and qtest test cases: unit tests simply test small parts of the code (that may or may not be hardware-specific), and qtest tests hardware emulation after starting up a whole qemu process. It's a different kind of testing, with different sets of goals. I suppose you are not arguing that no function inside target-* would be ever allowed to have a unit test written for it. That would be a very silly constraint for people writing tests. -- Eduardo
On Mon, Jul 23, 2012 at 8:14 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Jul 23, 2012 at 07:44:44PM +0000, Blue Swirl wrote:
On Mon, Jul 23, 2012 at 7:28 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Jul 23, 2012 at 07:11:11PM +0000, Blue Swirl wrote:
On Mon, Jul 23, 2012 at 6:59 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
On Mon, Jul 23, 2012 at 04:49:07PM +0000, Blue Swirl wrote:
On Mon, Jul 16, 2012 at 5:42 PM, Eduardo Habkost <ehabkost@redhat.com> wrote: > On Sat, Jul 14, 2012 at 09:14:30AM +0000, Blue Swirl wrote: > [...] >> >> > diff --git a/tests/Makefile b/tests/Makefile >> >> > index b605e14..89bd890 100644 >> >> > --- a/tests/Makefile >> >> > +++ b/tests/Makefile >> >> > @@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF) >> >> > check-unit-y += tests/test-coroutine$(EXESUF) >> >> > check-unit-y += tests/test-visitor-serialization$(EXESUF) >> >> > check-unit-y += tests/test-iov$(EXESUF) >> >> > +check-unit-y += tests/test-x86-cpuid$(EXESUF) >> >> >> >> This probably tries to build the cpuid test also for non-x86 targets >> >> and break them all. >> > >> > I don't think there's any concept of "targets" for the check-unit tests. >> >> How about: >> check-qtest-i386-y = tests/test-x86-cpuid$(EXESUF) > > test-x86-cpuid is not a qtest test case.
Why not? I don't think it is a unit test either, judging from what the other unit tests do.
It is absolutely a unit test. I don't know why you don't think so. It simply checks the results of the APIC ID calculation functions.
Yes, but the other 'unit tests' (the names used here are very confusing, btw) check supporting functions like coroutines, not hardware. Hardware tests (qtest) need to bind to an architecture, in this case x86.
test-x86-cpuid doesn't check hardware, either. It just checks the simple functions that calculate APIC IDs (to make sure the math is correct). Those functions aren't even used by any hardware emulation code until later in the patch series (when the CPU initialization code gets changed to use the new function).
By that time the function is clearly x86 HW and the check is an x86 hardware check. QEMU as whole consists of simple functions that calculate something.
It's useful onily for a specific architecture, yes, but it surely doesn't make libqtest necessary.
That's the difference between the unit tests and qtest test cases: unit tests simply test small parts of the code (that may or may not be hardware-specific), and qtest tests hardware emulation after starting up a whole qemu process. It's a different kind of testing, with different sets of goals.
Thanks for the clarification. I agree now that this is not a qtest. I think this is a new category of tests compared to those we have now: supporting function unit tests and hardware tests at device I/O boundary level.
I suppose you are not arguing that no function inside target-* would be ever allowed to have a unit test written for it. That would be a very silly constraint for people writing tests.
Of course. What I really want is that if x86 targets are not built, this test is skipped like qtests. This could be achieved with something like: check-unit-arch-i386-y = tests/test-x86-cpuid$(EXESUF) check-unit-y += $(foreach TARGET,$(TARGETS), $(check-unit-arch-$(TARGET)-y))
-- Eduardo
participants (2)
-
Blue Swirl -
Eduardo Habkost