Discussion:
[Swig-devel] How to deal with not yet working test cases?
Vadim Zeitlin
2016-04-13 16:05:06 UTC
Permalink
Hi,

I'm trying to bring yet another backend into shape for merge into SWIG
master, but right now I'm still very far from it and there are dozens
failing tests. I'd like to have a way to disable them when running the test
suite, so that I could at least check that my changes don't result in any
regressions, but unless I'm missing something, there doesn't seem to be any
way to do it now. I hoped that adding the failing tests to CPP_TEST_BROKEN
would do it, but this doesn't work like this as a test present both in
CPP_TEST_CASES and CPP_TEST_BROKEN still is getting run by default. I'm not
sure if this is intentional, but even if it is, I think it would be nice to
have some way of disabling the failing tests, so what about this trivial
patch:
---------------------------------- >8 --------------------------------------
diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk
index 3f408f3..e09cdbb 100644
--- a/Examples/test-suite/common.mk
+++ b/Examples/test-suite/common.mk
@@ -670,6 +670,11 @@ MULTI_CPP_TEST_CASES += \
wallkw.cpptest: SWIGOPT += -Wallkw
preproc_include.ctest: SWIGOPT += -includeall

+# Allow modules to define temporarily failing tests.
+C_TEST_CASES := $(filter-out $(FAILING_C_TESTS),$(C_TEST_CASES))
+CPP_TEST_CASES := $(filter-out $(FAILING_CPP_TESTS),$(CPP_TEST_CASES))
+MULTI_CPP_TEST_CASES := $(filter-out $(FAILING_MULTI_CPP_TESTS),$(MULTI_CPP_TEST_CASES))
+

NOT_BROKEN_TEST_CASES = $(CPP_TEST_CASES:=.cpptest) \
$(C_TEST_CASES:=.ctest) \
---------------------------------- >8 --------------------------------------

This would allow me to define FAILING_CPP_TESTS in the language-specific
makefile and skip testing them, at least temporarily. Or should we perhaps
just filter the broken test cases out? Or is there maybe some other, better
way of doing what I want already?

TIA,
VZ
William S Fulton
2016-04-13 19:53:28 UTC
Permalink
Post by Vadim Zeitlin
Hi,
I'm trying to bring yet another backend into shape for merge into SWIG
master, but right now I'm still very far from it and there are dozens
failing tests. I'd like to have a way to disable them when running the test
suite, so that I could at least check that my changes don't result in any
regressions, but unless I'm missing something, there doesn't seem to be any
way to do it now. I hoped that adding the failing tests to CPP_TEST_BROKEN
would do it, but this doesn't work like this as a test present both in
CPP_TEST_CASES and CPP_TEST_BROKEN still is getting run by default. I'm not
sure if this is intentional, but even if it is, I think it would be nice to
have some way of disabling the failing tests, so what about this trivial
---------------------------------- >8
--------------------------------------
diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk
index 3f408f3..e09cdbb 100644
--- a/Examples/test-suite/common.mk
+++ b/Examples/test-suite/common.mk
@@ -670,6 +670,11 @@ MULTI_CPP_TEST_CASES += \
wallkw.cpptest: SWIGOPT += -Wallkw
preproc_include.ctest: SWIGOPT += -includeall
+# Allow modules to define temporarily failing tests.
+C_TEST_CASES := $(filter-out $(FAILING_C_TESTS),$(C_TEST_CASES))
+CPP_TEST_CASES := $(filter-out $(FAILING_CPP_TESTS),$(CPP_TEST_CASES))
+MULTI_CPP_TEST_CASES := $(filter-out
$(FAILING_MULTI_CPP_TESTS),$(MULTI_CPP_TEST_CASES))
+
NOT_BROKEN_TEST_CASES = $(CPP_TEST_CASES:=.cpptest) \
$(C_TEST_CASES:=.ctest) \
---------------------------------- >8
--------------------------------------
This would allow me to define FAILING_CPP_TESTS in the language-specific
makefile and skip testing them, at least temporarily. Or should we perhaps
just filter the broken test cases out? Or is there maybe some other, better
way of doing what I want already?
I kind of like the fact that languages have to make all the testcases pass
and there is no clear way to avoid it! What I have seen while modules are
in development is to specify C_TEST_CASES etc after this line in the
language specific Makefile:

include $(srcdir)/../common.mk

C_TEST_CASES etc are added to as development progresses until they match
those in common.mk whereupon the Makefile can resort to the usual style. If
a language is skipping some tests, it really stands out, whereas having a
FAILING_C_TESTS variable doesn't.

Anyway, this isn't all that user friendly so if it helps, we can use your
patch unless you like going with what I described above.

William
Vadim Zeitlin
2016-04-13 20:20:44 UTC
Permalink
On Wed, 13 Apr 2016 20:53:28 +0100 William S Fulton <***@fultondesigns.co.uk> wrote:

WSF> I kind of like the fact that languages have to make all the testcases pass
WSF> and there is no clear way to avoid it!

This is the goal, of course, but it's not always possible to achieve it
immediately and having hundreds of errors every time you run the test suite
is just not very useful.

WSF> What I have seen while modules are in development is to specify
WSF> C_TEST_CASES etc after this line in the language specific Makefile:
WSF>
WSF> include $(srcdir)/../common.mk
WSF>
WSF> C_TEST_CASES etc are added to as development progresses until they match
WSF> those in common.mk whereupon the Makefile can resort to the usual style. If
WSF> a language is skipping some tests, it really stands out, whereas having a
WSF> FAILING_C_TESTS variable doesn't.

To be honest, I think defining FAILING_C_TESTS stands out much more
because you can grep for it and we could even output a warning message
about some tests having been skipped because they're known to fail.

And defining C_TEST_CASES in the language-specific makefile is a really
bad idea for long-living branches because the test will need to be updated
whenever a new test is added to common.mk. I.e. with this approach new
tests appearing on master will not be run by default after a merge into the
branch, while with my approach they will -- and will have to be explicitly
added to FAILING_C_TESTS if they fail.

Of course, I think ideal would be to reuse C_TEST_BROKEN for this instead
of having a different FAILING_C_TESTS, if only because it would also allow
running all the broken tests easily (I wonder if we could detect when a
broken test passes and give an error in this case, to indicate that it
should be removed from the BROKEN list). But I didn't want to modify any
existing makefiles magic...

WSF> Anyway, this isn't all that user friendly so if it helps, we can use your
WSF> patch unless you like going with what I described above.

If there are no other objections, I'll commit it to master in a couple of
days then.

Thanks,
VZ
William S Fulton
2016-04-14 06:22:23 UTC
Permalink
Post by Vadim Zeitlin
Of course, I think ideal would be to reuse C_TEST_BROKEN for this instead
of having a different FAILING_C_TESTS, if only because it would also allow
running all the broken tests easily (I wonder if we could detect when a
broken test passes and give an error in this case, to indicate that it
should be removed from the BROKEN list). But I didn't want to modify any
existing makefiles magic...
Probably a completely new and advanced approach to testing and reporting
is required, but this doesn't seem high priority to me given the never
ending list of improvements that could be done.
Post by Vadim Zeitlin
If there are no other objections, I'll commit it to master in a couple of
days then.
Okay.
William

Loading...