Page MenuHomeFreeBSD

multimedia/mpv: add TEST option
ClosedPublic

Authored by cpm on May 31 2017, 10:46 AM.

Details

Summary
- Add TEST option. Mpv includes a test suite executed with CMocka.
- Rename LIBARCHIVE option to ARCHIVE
- Cosmetic fixes
Test Plan
  • portlint -ac output looks fine.
  • poudriere testport builds fine on 103i386, 103amd64, 110i386, 110amd64, 120i386 and 120amd64.
  • make test has passed all unittests fine.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cpm created this revision.May 31 2017, 10:46 AM
cpm added a comment.May 31 2017, 10:48 AM

make test output:

===>  Testing for mpv-0.25.0_1,1
[==========] Running 1 test(s).
[ RUN      ] test_mp_chmap_diff
[       OK ] test_mp_chmap_diff
[==========] 1 test(s) run.
[  PASSED  ] 1 test(s).
[==========] Running 15 test(s).
[ RUN      ] test_mp_chmap_sel_fallback_upmix
[       OK ] test_mp_chmap_sel_fallback_upmix
[ RUN      ] test_mp_chmap_sel_fallback_downmix
[       OK ] test_mp_chmap_sel_fallback_downmix
[ RUN      ] test_mp_chmap_sel_fallback_incompatible
[       OK ] test_mp_chmap_sel_fallback_incompatible
[ RUN      ] test_mp_chmap_sel_fallback_prefer_compatible
[       OK ] test_mp_chmap_sel_fallback_prefer_compatible
[ RUN      ] test_mp_chmap_sel_fallback_prefer_closest_upmix
[       OK ] test_mp_chmap_sel_fallback_prefer_closest_upmix
[ RUN      ] test_mp_chmap_sel_fallback_use_replacements
[       OK ] test_mp_chmap_sel_fallback_use_replacements
[ RUN      ] test_mp_chmap_sel_fallback_inexact_equivalent
[       OK ] test_mp_chmap_sel_fallback_inexact_equivalent
[ RUN      ] test_mp_chmap_sel_fallback_works_on_alsa_chmaps
[       OK ] test_mp_chmap_sel_fallback_works_on_alsa_chmaps
[ RUN      ] test_mp_chmap_sel_fallback_mono_to_stereo
[       OK ] test_mp_chmap_sel_fallback_mono_to_stereo
[ RUN      ] test_mp_chmap_sel_fallback_stereo_to_stereo
[       OK ] test_mp_chmap_sel_fallback_stereo_to_stereo
[ RUN      ] test_mp_chmap_sel_fallback_no_downmix
[       OK ] test_mp_chmap_sel_fallback_no_downmix
[ RUN      ] test_mp_chmap_sel_fallback_minimal_downmix
[       OK ] test_mp_chmap_sel_fallback_minimal_downmix
[ RUN      ] test_mp_chmap_sel_fallback_reject_unknown
[       OK ] test_mp_chmap_sel_fallback_reject_unknown
[ RUN      ] test_mp_chmap_sel_fallback_more_replacements
[       OK ] test_mp_chmap_sel_fallback_more_replacements
[ RUN      ] test_mp_chmap_sel_fallback_na_channels
[       OK ] test_mp_chmap_sel_fallback_na_channels
[==========] 15 test(s) run.
[  PASSED  ] 15 test(s).
[==========] Running 4 test(s).
[ RUN      ] test_scale_ambient_lux_limits
[       OK ] test_scale_ambient_lux_limits
[ RUN      ] test_scale_ambient_lux_sign
[       OK ] test_scale_ambient_lux_sign
[ RUN      ] test_scale_ambient_lux_clamping
[       OK ] test_scale_ambient_lux_clamping
[ RUN      ] test_scale_ambient_lux_log10_midpoint
[       OK ] test_scale_ambient_lux_log10_midpoint
[==========] 4 test(s) run.
[  PASSED  ] 4 test(s).
cpm edited the summary of this revision. (Show Details)May 31 2017, 10:53 AM
cpm added a reviewer: jbeich.Jun 1 2017, 11:11 AM
cpm updated this revision to Diff 29174.Jun 3 2017, 3:26 PM
cpm edited the summary of this revision. (Show Details)

Update diff after r442496

jbeich edited edge metadata.Jun 15 2017, 9:17 PM

gl_video fails on 10.3 i386 or 11.0 i386 (poudriere jail) with 12.0 amd64 (host, drm-next).

[==========] Running 4 test(s).
[ RUN      ] test_scale_ambient_lux_limits
[  ERROR   ] --- fabs(x - 1.961f) <= DBL_EPSILON
[   LINE   ] --- ../test/gl_video.c:10: error: Failure!
[  FAILED  ] test_scale_ambient_lux_limits
[ RUN      ] test_scale_ambient_lux_sign
[  ERROR   ] --- fabs(x - 2.40f) <= DBL_EPSILON
[   LINE   ] --- ../test/gl_video.c:16: error: Failure!
[  FAILED  ] test_scale_ambient_lux_sign
[ RUN      ] test_scale_ambient_lux_clamping
[       OK ] test_scale_ambient_lux_clamping
[ RUN      ] test_scale_ambient_lux_log10_midpoint
[  ERROR   ] --- fabs(x - mid_gamma) <= DBL_EPSILON
[   LINE   ] --- ../test/gl_video.c:30: error: Failure!
[  FAILED  ] test_scale_ambient_lux_log10_midpoint
[==========] 4 test(s) run.
[  PASSED  ] 1 test(s).
[  FAILED  ] 3 test(s), listed below:
[  FAILED  ] test_scale_ambient_lux_limits
[  FAILED  ] test_scale_ambient_lux_sign
[  FAILED  ] test_scale_ambient_lux_log10_midpoint
multimedia/mpv/Makefile
60 ↗(On Diff #29174)

Maybe rename LIBARCHIVE to ARCHIVE and use _DESC from Mk/bsd.options.desc.mk.

62 ↗(On Diff #29174)

Doesn't explain more than TEST_DESC in Mk/bsd.options.desc.mk. Maybe drop.

130 ↗(On Diff #29174)

Use TEST_BUILD_DEPENDS = cmocka>0:sysutils/cmocka as you don't install test binaries.

131 ↗(On Diff #29174)

What is this for?

170 ↗(On Diff #29174)

Given TEST is a non-default option and poudriere#355 still hasn't landed maybe also add the following to always run tests if enabled. By using TEST=on for every release/arch jail you have it may help catching regressions early on updates, assuming you have time to keep the tests green.

pre-install-TEST-on: do-test-TEST-on
172 ↗(On Diff #29174)
  • Any reason for cd? Do you expect the tests to write/read something under current directory instead of using absolute paths?
  • Maybe drop @ symbol, so it's easier to tell different test collections apart in a build log.
cpm updated this revision to Diff 29691.Jun 16 2017, 12:31 AM
cpm edited the summary of this revision. (Show Details)
cpm edited the test plan for this revision. (Show Details)
cpm edited the summary of this revision. (Show Details)
cpm marked 2 inline comments as done.
cpm marked an inline comment as done.Jun 16 2017, 12:34 AM
cpm added inline comments.Jun 16 2017, 12:44 AM
multimedia/mpv/Makefile
130 ↗(On Diff #29174)

No, it needs libcmocka.so to run unittest. Furthermore sysutils/cmocka doesn't include cmocka binary.

131 ↗(On Diff #29174)

Mpv is unable to find libcmocka.pc in the default PKG_CONFIG_PATH.

172 ↗(On Diff #29174)

Well, another way to run it might look like this

.for test in ${TEST_UNITTESTFILES} 
     ${WRKSRC}/build/tests/${test}
.endfor
cpm added a comment.Jun 16 2017, 12:51 AM

gl_video fails on 10.3 i386 or 11.0 i386 (poudriere jail) with 12.0 amd64 (host, drm-next).

[==========] Running 4 test(s).
[ RUN      ] test_scale_ambient_lux_limits
[  ERROR   ] --- fabs(x - 1.961f) <= DBL_EPSILON
[   LINE   ] --- ../test/gl_video.c:10: error: Failure!
[  FAILED  ] test_scale_ambient_lux_limits
[ RUN      ] test_scale_ambient_lux_sign
[  ERROR   ] --- fabs(x - 2.40f) <= DBL_EPSILON
[   LINE   ] --- ../test/gl_video.c:16: error: Failure!
[  FAILED  ] test_scale_ambient_lux_sign
[ RUN      ] test_scale_ambient_lux_clamping
[       OK ] test_scale_ambient_lux_clamping
[ RUN      ] test_scale_ambient_lux_log10_midpoint
[  ERROR   ] --- fabs(x - mid_gamma) <= DBL_EPSILON
[   LINE   ] --- ../test/gl_video.c:30: error: Failure!
[  FAILED  ] test_scale_ambient_lux_log10_midpoint
[==========] 4 test(s) run.
[  PASSED  ] 1 test(s).
[  FAILED  ] 3 test(s), listed below:
[  FAILED  ] test_scale_ambient_lux_limits
[  FAILED  ] test_scale_ambient_lux_sign
[  FAILED  ] test_scale_ambient_lux_log10_midpoint

The problem is the assertion defined in test/test_helpers.h

https://github.com/mpv-player/mpv/commit/c028d782c1eec46e2416da483881f1d0b27c2be8#diff-38e81292b049e7e898962ee4160f06abR13

cpm updated this revision to Diff 29692.Jun 16 2017, 12:58 AM
  • Add TEST_UNITTESTFILES option variable for tests.
jbeich added inline comments.Jun 16 2017, 1:12 AM
multimedia/mpv/Makefile
130 ↗(On Diff #29174)
  • Why mpv (not the unittest) links against libcmocka.so without using any of its symbols? Looks like an overlinking bug. LDFLAGS+=-Wl,--as-needed may help but depends on the flag order.
  • cmocka>0 syntax is to require a specific version of the package (not binary) but in the example we ignore version.
131 ↗(On Diff #29174)

rP442784 moved libcmocka.pc under libdata/pkgconfig. Maybe upgrade. Can you reproduce via poudriere + TEST=on?

cpm updated this revision to Diff 29694.Jun 16 2017, 1:24 AM
  • Use TEST_BUILD_DEPENDS
cpm marked 2 inline comments as done.Jun 16 2017, 1:24 AM
cpm added inline comments.Jun 16 2017, 1:27 AM
multimedia/mpv/Makefile
131 ↗(On Diff #29174)

You're right! Rebuilding cmocka port did the trick.

cpm updated this revision to Diff 29696.Jun 16 2017, 1:29 AM
  • Fix cmocka version required.
cpm updated this revision to Diff 29698.Jun 16 2017, 3:10 AM
  • Use FLT_EPSILON constant to fix gl_video unittest on i386
cpm edited the test plan for this revision. (Show Details)Jun 16 2017, 3:12 AM
This revision was automatically updated to reflect the committed changes.