The following patches fix Salut's tests infrastructure. Note that some tests are failing because of specific issues, I'll open other bug(s) for those. I tested with and without SALUT_TEST_REAL_AVAHI.
Created attachment 85615 [details] [review] .gitignore: automake 1.13 copies in /test-driver
Created attachment 85616 [details] [review] configure: use same debug check code than Gabble
Created attachment 85617 [details] [review] configure: add summary
Created attachment 85618 [details] [review] move sidecars.py to TWISTED_AVAHI_TESTS
Created attachment 85619 [details] [review] Import run-test.sh.in from Gabble master
Created attachment 85620 [details] [review] Run regression tests under the run-test.sh "driver"
Created attachment 85621 [details] [review] always enable Avahi tests when building with the Avahi backend
Comment on attachment 85616 [details] [review] configure: use same debug check code than Gabble Review of attachment 85616 [details] [review]: ----------------------------------------------------------------- Did we not have AC_ARG_ENABLE(debug) somewhere else already? Strictly speaking, this is underquoted (AC_ARG_ENABLE([debug], ...) - one level of [] per level of ()). The if/fi should remain an AS_IF, which doesn't matter except for when it does (Bug #53445, <https://bugzilla.gnome.org/show_bug.cgi?id=681413>).
Comment on attachment 85617 [details] [review] configure: add summary Review of attachment 85617 [details] [review]: ----------------------------------------------------------------- If you like. I find these summaries a bit spammy - they're working around the amount of noise configure generates by adding more noise - but whatever.
Comment on attachment 85620 [details] [review] Run regression tests under the run-test.sh "driver" Review of attachment 85620 [details] [review]: ----------------------------------------------------------------- ::: tests/twisted/Makefile.am @@ +115,5 @@ > +# so far - but I'm substituting it to keep the script more similar to Gabble's. > +# ${pkglibexecdir}/tests is what GNOME's InstalledTests goal recommends. > +# > +# Similarly, Gabble supports TEST_PYTHON differing from PYTHON for historical > +# reasons, but we don't do that here. Are you sure about that? configure mentions TEST_PYTHON.
All good except where noted.
(In reply to comment #8) > Comment on attachment 85616 [details] [review] [review] > configure: use same debug check code than Gabble > > Review of attachment 85616 [details] [review] [review]: > ----------------------------------------------------------------- > > Did we not have AC_ARG_ENABLE(debug) somewhere else already? No, actually that's the thing I meant to fix. Patch updated.
(In reply to comment #9) > Comment on attachment 85617 [details] [review] [review] > configure: add summary > > Review of attachment 85617 [details] [review] [review]: > ----------------------------------------------------------------- > > If you like. I find these summaries a bit spammy - they're working around > the amount of noise configure generates by adding more noise - but whatever. I actually like them, if only when debugging/hacking on configure.ac.
(In reply to comment #10) > Comment on attachment 85620 [details] [review] [review] > Run regression tests under the run-test.sh "driver" > > Review of attachment 85620 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: tests/twisted/Makefile.am > @@ +115,5 @@ > > +# so far - but I'm substituting it to keep the script more similar to Gabble's. > > +# ${pkglibexecdir}/tests is what GNOME's InstalledTests goal recommends. > > +# > > +# Similarly, Gabble supports TEST_PYTHON differing from PYTHON for historical > > +# reasons, but we don't do that here. > > Are you sure about that? configure mentions TEST_PYTHON. You're right, I just re-used your file. I removed this comment.
Created attachment 85626 [details] [review] configure: add AC_ARG_ENABLE(debug)
Created attachment 85627 [details] [review] Run regression tests under the run-test.sh "driver"
Comment on attachment 85627 [details] [review] Run regression tests under the run-test.sh "driver" Review of attachment 85627 [details] [review]: ----------------------------------------------------------------- ::: tests/twisted/Makefile.am @@ +116,5 @@ > +# ${pkglibexecdir}/tests is what GNOME's InstalledTests goal recommends. > +run-test.sh: run-test.sh.in Makefile > + $(AM_V_GEN)sed \ > + -e 's![@]saluttestsdir[@]!${pkglibexecdir}/tests!' \ > + -e 's![@]TEST_PYTHON[@]!$(PYTHON)!' \ Replace with $(TEST_PYTHON) please? Other than that, ++
Comment on attachment 85626 [details] [review] configure: add AC_ARG_ENABLE(debug) Review of attachment 85626 [details] [review]: ----------------------------------------------------------------- ++ with comments: ::: configure.ac @@ +89,4 @@ > unused-parameter]) > AC_SUBST([ERROR_CFLAGS]) > > +AC_ARG_ENABLE(debug, [debug] would be better, but not very important @@ +90,5 @@ > AC_SUBST([ERROR_CFLAGS]) > > +AC_ARG_ENABLE(debug, > + AC_HELP_STRING([--disable-debug],[compile without debug code]), > + enable_debug=$enableval, enable_debug=yes ) Would be better as "[enable_debug=$enableval], [enable_debug=yes])" or even "[], [enable_debug=yes])" although it isn't important. (Look at the generated code - AC_ARG_ENABLE([foo]) always sets enable_foo in the "user-specified" case anyway.)
(In reply to comment #18) > > +AC_ARG_ENABLE(debug, > > + AC_HELP_STRING([--disable-debug],[compile without debug code]), > > + enable_debug=$enableval, enable_debug=yes ) Less misleading indentation would also be nice: enable_debug=... is at the same level as AC_HELP_STRING. Strictly speaking, the whole AC_HELP_STRING call should also be inside [] to avoid underquoting: AC_ARG_ENABLE([debug], [AC_HELP_STRING([--disable-debug], [compile without debug code])], [], [enable_debug=yes])
(In reply to comment #17) > Comment on attachment 85627 [details] [review] [review] > Run regression tests under the run-test.sh "driver" > > Review of attachment 85627 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: tests/twisted/Makefile.am > @@ +116,5 @@ > > +# ${pkglibexecdir}/tests is what GNOME's InstalledTests goal recommends. > > +run-test.sh: run-test.sh.in Makefile > > + $(AM_V_GEN)sed \ > > + -e 's![@]saluttestsdir[@]!${pkglibexecdir}/tests!' \ > > + -e 's![@]TEST_PYTHON[@]!$(PYTHON)!' \ > > Replace with $(TEST_PYTHON) please? Other than that, ++ done. In reply to comment #19) > (In reply to comment #18) > > > +AC_ARG_ENABLE(debug, > > > + AC_HELP_STRING([--disable-debug],[compile without debug code]), > > > + enable_debug=$enableval, enable_debug=yes ) > > Less misleading indentation would also be nice: enable_debug=... is at the > same level as AC_HELP_STRING. > > Strictly speaking, the whole AC_HELP_STRING call should also be inside [] to > avoid underquoting: > > AC_ARG_ENABLE([debug], > [AC_HELP_STRING([--disable-debug], [compile without debug code])], > [], [enable_debug=yes]) changed.
Merged to master; thanks!
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.