Bug 203514 - devel/googletest, devel/googlemock: fix tests, fix python cmd handling
Summary: devel/googletest, devel/googlemock: fix tests, fix python cmd handling
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Jan Beich
Depends on:
Reported: 2015-10-02 21:04 UTC by Dmitry Marakasov
Modified: 2015-10-03 01:34 UTC (History)
1 user (show)

See Also:
jbeich: maintainer-feedback+

Patch (2.79 KB, patch)
2015-10-02 21:05 UTC, Dmitry Marakasov
amdmi3: maintainer-approval? (jbeich)
Details | Diff
maintainer version (1.94 KB, patch)
2015-10-02 23:28 UTC, Jan Beich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Marakasov freebsd_committer 2015-10-02 21:04:47 UTC
- Silence patching
- Move all python-related knobs under TEST target (including shebangfix and CONFIGURE_ENV
- Use correct python command

With googlemock, additionally fix tests in a different ways:
- Tests require googletest, make it available to them
- Additional test fails unless it's linked with -lpthread

Actually these should be converted onto new test framework, but as there's no convenient way to set TEST_TARGET with an option (https://reviews.freebsd.org/D3788) I'm leaving this as is
Comment 1 Dmitry Marakasov freebsd_committer 2015-10-02 21:05:16 UTC
Created attachment 161659 [details]
Comment 2 Dmitry Marakasov freebsd_committer 2015-10-02 23:14:33 UTC
Just thought that TEST_TARGET may become unconditional, as soon as ports are taught to NOT detect any python when TEST is disabled (they'll break otherwise). Will work on this.
Comment 3 Jan Beich freebsd_committer 2015-10-02 23:28:41 UTC
Created attachment 161664 [details]
maintainer version

My idea of the proper fix is attached. Sorry, most of your changes ended up discarded. I haven't QA'd it yet. ;)

>-CONFIGURE_ENV=	ac_cv_path_PYTHON=python2
>-python_CMD=	/usr/bin/env python2
>+TEST_USES=	python:2,build shebangfix

This has a few issues:

- PYTHON_CMD specifies fully-qualified python version, so you don't
  need lang/python2 dependency -> USES=python:2 vs. USES=python:-2.7
- HAVE_PYTHON is expanded to a random value when TESTS is disabled;
  fortunately, the enabled code is only invoked by |check| target

>+	@${CP} -R `${MAKE} -C ${PORTSDIR}/devel/googletest -V WRKSRC` ${WRKSRC}/gtest
>+@@ -85,6 +85,7 @@ if HAVE_PYTHON
>+     fused-src/gtest/gtest.h \
>+     test/gmock_test.cc
>+   test_gmock_fused_test_CPPFLAGS = -I"$(srcdir)/fused-src"
>++  test_gmock_fused_test_LDFLAGS = -lpthread

Enabling python stuff was probably a mistake. System gtest/gmock don't ship the source code that can be fused, so no point in testing it and complicating the port.

gmock_doctor.py still needs |/usr/bin/env python2| shebang as it won't
run with /usr/local/bin/python when USES=python is not provided.
Comment 4 Jan Beich freebsd_committer 2015-10-02 23:32:36 UTC
Note, I don't mind dropping USES=shebangfix except for gmock_doctor.py if you think the current situation is confusing.
Comment 5 Jan Beich freebsd_committer 2015-10-02 23:35:51 UTC
Comment on attachment 161664 [details]
maintainer version

Oops, forgot to copy @ for post-patch in googlemock from your version.
Comment 6 Dmitry Marakasov freebsd_committer 2015-10-02 23:48:13 UTC
Looks good to me except:

- You don't need shebangfix files which are not called or installed. So, you only need to shebangfix gmock_doctor.py in googlemock, not python is needed for googletest
- You don't need python_CMD
Comment 7 commit-hook freebsd_committer 2015-10-03 01:32:07 UTC
A commit references this bug:

Author: jbeich
Date: Sat Oct  3 01:32:00 UTC 2015
New revision: 398458
URL: https://svnweb.freebsd.org/changeset/ports/398458

  devel/google{test,mock}: simplify and modernize

  - Convert to the new testing framework
  - Add upstream fix for gmock_doctor.py to work with python3, so
    keep existing |/usr/bin/env python| shebang
  - Drop fused-src tests as the ports don't install source files
  - No need to shebangfix files we don't install
  - Provide LICENSE_FILE as there's none under Templates/Licenses/
  - Sort declarations by the order they're run (ldconfig is last)
  - Silence post-patch commands [1]

  PR:		203514
  Submitted by:	amdmi3 [1]
  Reviewed by:	amdmi3 (previous version)

Comment 8 Jan Beich freebsd_committer 2015-10-03 01:34:04 UTC
Done. Lightly tested on devel/rlvm with TEST=on which is partly broken for other reasons.