| Message ID | 1452704893-17874-1-git-send-email-christophe.milard@linaro.org |
|---|---|
| State | Accepted |
| Commit | 43625d21d3afabf4022f49e470cbe3e1e45ea3fc |
| Headers | show |
This seems like a reasonable addition, but we need to consider carefully what conditions are valid for skipping tests, otherwise it makes it too easy for ODP APIs to become optional. Can we include some guidelines here? Especially given that we have the ability to support platform-specific tests in addition to global tests, we should clarify under what conditions a global test would be made conditional vs. just having a platform-specific test for a feature that may be unique to a particular platform. On Wed, Jan 13, 2016 at 11:08 AM, Christophe Milard < christophe.milard@linaro.org> wrote: > Documentation on conditional test is added here, as it seemed to be missing > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > --- > doc/implementers-guide/implementers-guide.adoc | 29 > ++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/doc/implementers-guide/implementers-guide.adoc > b/doc/implementers-guide/implementers-guide.adoc > index eb076cf..57be4b2 100644 > --- a/doc/implementers-guide/implementers-guide.adoc > +++ b/doc/implementers-guide/implementers-guide.adoc > @@ -389,4 +389,33 @@ It's expected that early in the development cycle of > a new implementation the > inactive list will be quite long, but it should shrink over time as more > parts > of the API are implemented. > > +==== conditionnal tests ==== > + > +Some tests may require specific conditions to make sense: for instance, on > +pktio, checking that sending a packet larger than the MTU is rejected > only makes > +sense if packets can indeed, on that ODP implementation, exceed the MTU. > +A test can be marked conditionnal as follows: > + > +[source,c] > +------------------ > +odp_testinfo_t foo_tests[] = { > + ... > + ODP_TEST_INFO_CONDITIONAL(foo_test_x, foo_check_x), > + ... > + ODP_TEST_INFO_NULL > +}; > + > +odp_suiteinfo_t foo_suites[] = { > + {"Foo", foo_suite_init, foo_suite_term, foo_tests}, > + ODP_SUITE_INFO_NULL > +}; > +------------------ > + > +Foo_test_x is the usual test function. Foo_check_x is the test > precondition, > +i.e. a function returning an bollean (int). > +It is called before the test suite is started. If it returns true, the > +test (foo_test_x) is run. If the precondition function (foo_check_x above) > +returns false, the test is not relevant (or impossible to perform) and it > will > +be skipped. > + > include::../glossary.adoc[] > -- > 2.1.4 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
The implementers guide says: "The general policy is that a full run of the validation suite (a "make check") must pass at all times. However a particular platform may have one or more test cases that are known to be unimplemented either during development or permanently, so to avoid these test cases being reported as failures it’s useful to be able to skip them. This can be achieved by creating a new test executable (still on the platform side), giving the platform specific initialization code the opportunity to modify the registered tests in order to mark unwanted tests as inactive while leaving the remaining tests active. It’s important that the unwanted tests are still registered with the test framework to allow the fact that they’re not being tested to be recorded." Moreover, this patch does not make tests deactivation more or less accessible: it just makes the syntax easier: the result being that there will be more chance that people mark the test as inactive (which is shown in the test results) rather than suppressing it simply (Which is not shown). I agree, I can see problems defining ODP compatibility in the future if different implementations supports different subset of the API (We already see a bit of that with the cache coherency functions that Nicolas will add and that will only be supported by these kinds of systems.). Maybe this has to be discussed in an ARCH call and possibly the XML file generation from C-UNIT considered again. There was also discussions about defining different ODP "profiles" when I restructured the test environment... or we simply need documentation updates. I don't think this patch should be stopped for that anyway: Marking tests as inactive is already an available feature: simplifying its syntax makes sense to me. Christophe. On 25 January 2016 at 02:53, Bill Fischofer <bill.fischofer@linaro.org> wrote: > This seems like a reasonable addition, but we need to consider carefully > what conditions are valid for skipping tests, otherwise it makes it too > easy for ODP APIs to become optional. Can we include some guidelines > here? Especially given that we have the ability to support > platform-specific tests in addition to global tests, we should clarify > under what conditions a global test would be made conditional vs. just > having a platform-specific test for a feature that may be unique to a > particular platform. > > On Wed, Jan 13, 2016 at 11:08 AM, Christophe Milard < > christophe.milard@linaro.org> wrote: > >> Documentation on conditional test is added here, as it seemed to be >> missing >> >> Signed-off-by: Christophe Milard <christophe.milard@linaro.org> >> --- >> doc/implementers-guide/implementers-guide.adoc | 29 >> ++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/doc/implementers-guide/implementers-guide.adoc >> b/doc/implementers-guide/implementers-guide.adoc >> index eb076cf..57be4b2 100644 >> --- a/doc/implementers-guide/implementers-guide.adoc >> +++ b/doc/implementers-guide/implementers-guide.adoc >> @@ -389,4 +389,33 @@ It's expected that early in the development cycle of >> a new implementation the >> inactive list will be quite long, but it should shrink over time as more >> parts >> of the API are implemented. >> >> +==== conditionnal tests ==== >> + >> +Some tests may require specific conditions to make sense: for instance, >> on >> +pktio, checking that sending a packet larger than the MTU is rejected >> only makes >> +sense if packets can indeed, on that ODP implementation, exceed the MTU. >> +A test can be marked conditionnal as follows: >> + >> +[source,c] >> +------------------ >> +odp_testinfo_t foo_tests[] = { >> + ... >> + ODP_TEST_INFO_CONDITIONAL(foo_test_x, foo_check_x), >> + ... >> + ODP_TEST_INFO_NULL >> +}; >> + >> +odp_suiteinfo_t foo_suites[] = { >> + {"Foo", foo_suite_init, foo_suite_term, foo_tests}, >> + ODP_SUITE_INFO_NULL >> +}; >> +------------------ >> + >> +Foo_test_x is the usual test function. Foo_check_x is the test >> precondition, >> +i.e. a function returning an bollean (int). >> +It is called before the test suite is started. If it returns true, the >> +test (foo_test_x) is run. If the precondition function (foo_check_x >> above) >> +returns false, the test is not relevant (or impossible to perform) and >> it will >> +be skipped. >> + >> include::../glossary.adoc[] >> -- >> 2.1.4 >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> > >
Good points. As long as we're disciplined about this I agree the cleaner syntax makes sense. Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> On Mon, Jan 25, 2016 at 1:43 AM, Christophe Milard < christophe.milard@linaro.org> wrote: > The implementers guide says: > "The general policy is that a full run of the validation suite (a "make > check") must pass at all times. However a particular platform may have one > or more test cases that are known to be unimplemented either during > development or permanently, so to avoid these test cases being reported as > failures it’s useful to be able to skip them. This can be achieved by > creating a new test executable (still on the platform side), giving the > platform specific initialization code the opportunity to modify the > registered tests in order to mark unwanted tests as inactive while leaving > the remaining tests active. It’s important that the unwanted tests are > still registered with the test framework to allow the fact that they’re not > being tested to be recorded." > > Moreover, this patch does not make tests deactivation more or less > accessible: it just makes the syntax easier: the result being that there > will be more chance that people mark the test as inactive (which is shown > in the test results) rather than suppressing it simply (Which is not shown). > > I agree, I can see problems defining ODP compatibility in the future if > different implementations supports different subset of the API (We already > see a bit of that with the cache coherency functions that Nicolas will add > and that will only be supported by these kinds of systems.). > > Maybe this has to be discussed in an ARCH call and possibly the XML file > generation from C-UNIT considered again. There was also discussions about > defining different ODP "profiles" when I restructured the test > environment... or we simply need documentation updates. > > I don't think this patch should be stopped for that anyway: Marking tests > as inactive is already an available feature: simplifying its syntax makes > sense to me. > > Christophe. > > > > > > > > On 25 January 2016 at 02:53, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> This seems like a reasonable addition, but we need to consider carefully >> what conditions are valid for skipping tests, otherwise it makes it too >> easy for ODP APIs to become optional. Can we include some guidelines >> here? Especially given that we have the ability to support >> platform-specific tests in addition to global tests, we should clarify >> under what conditions a global test would be made conditional vs. just >> having a platform-specific test for a feature that may be unique to a >> particular platform. >> >> On Wed, Jan 13, 2016 at 11:08 AM, Christophe Milard < >> christophe.milard@linaro.org> wrote: >> >>> Documentation on conditional test is added here, as it seemed to be >>> missing >>> >>> Signed-off-by: Christophe Milard <christophe.milard@linaro.org> >>> --- >>> doc/implementers-guide/implementers-guide.adoc | 29 >>> ++++++++++++++++++++++++++ >>> 1 file changed, 29 insertions(+) >>> >>> diff --git a/doc/implementers-guide/implementers-guide.adoc >>> b/doc/implementers-guide/implementers-guide.adoc >>> index eb076cf..57be4b2 100644 >>> --- a/doc/implementers-guide/implementers-guide.adoc >>> +++ b/doc/implementers-guide/implementers-guide.adoc >>> @@ -389,4 +389,33 @@ It's expected that early in the development cycle >>> of a new implementation the >>> inactive list will be quite long, but it should shrink over time as >>> more parts >>> of the API are implemented. >>> >>> +==== conditionnal tests ==== >>> + >>> +Some tests may require specific conditions to make sense: for instance, >>> on >>> +pktio, checking that sending a packet larger than the MTU is rejected >>> only makes >>> +sense if packets can indeed, on that ODP implementation, exceed the MTU. >>> +A test can be marked conditionnal as follows: >>> + >>> +[source,c] >>> +------------------ >>> +odp_testinfo_t foo_tests[] = { >>> + ... >>> + ODP_TEST_INFO_CONDITIONAL(foo_test_x, foo_check_x), >>> + ... >>> + ODP_TEST_INFO_NULL >>> +}; >>> + >>> +odp_suiteinfo_t foo_suites[] = { >>> + {"Foo", foo_suite_init, foo_suite_term, foo_tests}, >>> + ODP_SUITE_INFO_NULL >>> +}; >>> +------------------ >>> + >>> +Foo_test_x is the usual test function. Foo_check_x is the test >>> precondition, >>> +i.e. a function returning an bollean (int). >>> +It is called before the test suite is started. If it returns true, the >>> +test (foo_test_x) is run. If the precondition function (foo_check_x >>> above) >>> +returns false, the test is not relevant (or impossible to perform) and >>> it will >>> +be skipped. >>> + >>> include::../glossary.adoc[] >>> -- >>> 2.1.4 >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> https://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> >
Merged, Maxim. On 01/25/2016 15:57, Bill Fischofer wrote: > Good points. As long as we're disciplined about this I agree the > cleaner syntax makes sense. > > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> > > On Mon, Jan 25, 2016 at 1:43 AM, Christophe Milard > <christophe.milard@linaro.org <mailto:christophe.milard@linaro.org>> > wrote: > > The implementers guide says: > "The general policy is that a full run of the validation suite (a > "make check") must pass at all times. However a particular > platform may have one or more test cases that are known to be > unimplemented either during development or permanently, so to > avoid these test cases being reported as failures it’s useful to > be able to skip them. This can be achieved by creating a new test > executable (still on the platform side), giving the platform > specific initialization code the opportunity to modify the > registered tests in order to mark unwanted tests as inactive while > leaving the remaining tests active. It’s important that the > unwanted tests are still registered with the test framework to > allow the fact that they’re not being tested to be recorded." > > Moreover, this patch does not make tests deactivation more or less > accessible: it just makes the syntax easier: the result being that > there will be more chance that people mark the test as inactive > (which is shown in the test results) rather than suppressing it > simply (Which is not shown). > > I agree, I can see problems defining ODP compatibility in the > future if different implementations supports different subset of > the API (We already see a bit of that with the cache coherency > functions that Nicolas will add and that will only be supported by > these kinds of systems.). > > Maybe this has to be discussed in an ARCH call and possibly the > XML file generation from C-UNIT considered again. There was also > discussions about defining different ODP "profiles" when I > restructured the test environment... or we simply need > documentation updates. > > I don't think this patch should be stopped for that anyway: > Marking tests as inactive is already an available feature: > simplifying its syntax makes sense to me. > > Christophe. > > > > > > > > On 25 January 2016 at 02:53, Bill Fischofer > <bill.fischofer@linaro.org <mailto:bill.fischofer@linaro.org>> wrote: > > This seems like a reasonable addition, but we need to consider > carefully what conditions are valid for skipping tests, > otherwise it makes it too easy for ODP APIs to become > optional. Can we include some guidelines here? Especially > given that we have the ability to support platform-specific > tests in addition to global tests, we should clarify under > what conditions a global test would be made conditional vs. > just having a platform-specific test for a feature that may be > unique to a particular platform. > > On Wed, Jan 13, 2016 at 11:08 AM, Christophe Milard > <christophe.milard@linaro.org > <mailto:christophe.milard@linaro.org>> wrote: > > Documentation on conditional test is added here, as it > seemed to be missing > > Signed-off-by: Christophe Milard > <christophe.milard@linaro.org > <mailto:christophe.milard@linaro.org>> > --- > doc/implementers-guide/implementers-guide.adoc | 29 > ++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git > a/doc/implementers-guide/implementers-guide.adoc > b/doc/implementers-guide/implementers-guide.adoc > index eb076cf..57be4b2 100644 > --- a/doc/implementers-guide/implementers-guide.adoc > +++ b/doc/implementers-guide/implementers-guide.adoc > @@ -389,4 +389,33 @@ It's expected that early in the > development cycle of a new implementation the > inactive list will be quite long, but it should shrink > over time as more parts > of the API are implemented. > > +==== conditionnal tests ==== > + > +Some tests may require specific conditions to make sense: > for instance, on > +pktio, checking that sending a packet larger than the MTU > is rejected only makes > +sense if packets can indeed, on that ODP implementation, > exceed the MTU. > +A test can be marked conditionnal as follows: > + > +[source,c] > +------------------ > +odp_testinfo_t foo_tests[] = { > + ... > + ODP_TEST_INFO_CONDITIONAL(foo_test_x, foo_check_x), > + ... > + ODP_TEST_INFO_NULL > +}; > + > +odp_suiteinfo_t foo_suites[] = { > + {"Foo", foo_suite_init, foo_suite_term, foo_tests}, > + ODP_SUITE_INFO_NULL > +}; > +------------------ > + > +Foo_test_x is the usual test function. Foo_check_x is the > test precondition, > +i.e. a function returning an bollean (int). > +It is called before the test suite is started. If it > returns true, the > +test (foo_test_x) is run. If the precondition function > (foo_check_x above) > +returns false, the test is not relevant (or impossible to > perform) and it will > +be skipped. > + > include::../glossary.adoc[] > -- > 2.1.4 > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > > > > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/doc/implementers-guide/implementers-guide.adoc b/doc/implementers-guide/implementers-guide.adoc index eb076cf..57be4b2 100644 --- a/doc/implementers-guide/implementers-guide.adoc +++ b/doc/implementers-guide/implementers-guide.adoc @@ -389,4 +389,33 @@ It's expected that early in the development cycle of a new implementation the inactive list will be quite long, but it should shrink over time as more parts of the API are implemented. +==== conditionnal tests ==== + +Some tests may require specific conditions to make sense: for instance, on +pktio, checking that sending a packet larger than the MTU is rejected only makes +sense if packets can indeed, on that ODP implementation, exceed the MTU. +A test can be marked conditionnal as follows: + +[source,c] +------------------ +odp_testinfo_t foo_tests[] = { + ... + ODP_TEST_INFO_CONDITIONAL(foo_test_x, foo_check_x), + ... + ODP_TEST_INFO_NULL +}; + +odp_suiteinfo_t foo_suites[] = { + {"Foo", foo_suite_init, foo_suite_term, foo_tests}, + ODP_SUITE_INFO_NULL +}; +------------------ + +Foo_test_x is the usual test function. Foo_check_x is the test precondition, +i.e. a function returning an bollean (int). +It is called before the test suite is started. If it returns true, the +test (foo_test_x) is run. If the precondition function (foo_check_x above) +returns false, the test is not relevant (or impossible to perform) and it will +be skipped. + include::../glossary.adoc[]
Documentation on conditional test is added here, as it seemed to be missing Signed-off-by: Christophe Milard <christophe.milard@linaro.org> --- doc/implementers-guide/implementers-guide.adoc | 29 ++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)