| Summary: | app-office/gnucash-2.7.4: multiple test failures | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Thomas Deutschmann (RETIRED) <whissi> |
| Component: | Current packages | Assignee: | Aaron W. Swenson <titanofold> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | gnome |
| Priority: | Normal | Keywords: | TESTFAILURE |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | https://github.com/Gnucash/gnucash/pull/285 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Attachments: |
build.log
LastTest.log LastTest.log -r1 skip test-gnc-timezone test for now gnucash-x86-tests-fix.patch |
||
|
Description
Thomas Deutschmann (RETIRED)
2018-02-14 13:35:52 UTC
Created attachment 519412 [details]
LastTest.log
(In reply to Thomas Deutschmann from comment #0) > The following tests FAILED: > 24 - test-userdata-dir-invalid-home (Failed) > 32 - test-qof (OTHER_FAULT) > 55 - test-gnc-numeric (Failed) > 56 - test-gnc-timezone (Failed) > Errors while running CTest I'm working on figuring out the failures. These seem to be Gentoo-specific failures. test-qof is resolved by having the following in /etc/locale.gen: en_GB.UTF-8 UTF-8 en_US.UTF-8 UTF-8 fr_FR.UTF-8 UTF-8 Okay, so adding those locales took care of the other tests, I think. At least, numeric and timezone aren't failing for me. However, I'm now getting an invalid pointer error from test-userdata-dir which in turn means test-userdata-dir-invalid-home fails. Further still, if I just navigate to the build directory and invoke "make check", everything passes. Portage does various environment var setup that you aren't getting by manually going in that dir and running it. Additionally it runs it all under sandbox, which you can partially emulate via /usr/bin/sandbox, but not fully I think (portage sets some things up for further restrictions or allowances iirc). Tests shouldn't depend on locales I think; For upstream the test should automatically skip if the locale isn't present, I believe. Might be easier to skip it for all downstream though until it's fixed fully upstream. Though given the urgency of all this for webkit-gtk/icu, I think it'd be fine to require the locales for current fast stabling and fix it up later on properly. Though you have some other failures still I guess. (In reply to Mart Raudsepp from comment #4) > Might be easier > to skip it for all downstream though until it's fixed fully upstream. Just to be clear - here I meant only disabling these specific tests, not a full RESTRICT=test I understood what you meant.
I've figured out all the problems.
In addition to the locale.gen changes and a patch to take care of a double-free, src_test() needs:
cp common/test-core/unittest_support.py \
"${BUILD_DIR}"/common/test-core/ || die;
cd "${BUILD_DIR}" || die;
XDG_DATA_HOME="${T}/$(whoami)" emake check
I haven't figured out how to skip particular tests, yet.
Is this acceptable for now? I'm going to raise the missing locale causes test failure issue upstream.
(In reply to Aaron W. Swenson from comment #6) > I understood what you meant. > > I've figured out all the problems. > > In addition to the locale.gen changes and a patch to take care of a > double-free, src_test() needs: > > cp common/test-core/unittest_support.py \ > "${BUILD_DIR}"/common/test-core/ || die; > cd "${BUILD_DIR}" || die; > XDG_DATA_HOME="${T}/$(whoami)" emake check Odd, doesn't it work with xdg_environment_reset already from pkg_setup that sets XDG_DATA_HOME to "${HOME}/.local/share" (HOME is in $PORTAGE_BUILDDIR/homedir)? Is that for some reason ineffective in src_test or it needs $(whoami) as the subdir name for some odd reason? If that's the case, I'd try to use ${USER} for now instead of $(whoami), or whatever is appropriate from bash and the env. > I haven't figured out how to skip particular tests, yet. > > Is this acceptable for now? I'm going to raise the missing locale causes > test failure issue upstream. I'd say it's not ideal, but if you state clearly what locales need to be present for the tests to pass, it ought to be fine for getting it done for webkit-gtk riddance needs (and stable really being broken as-is). (In reply to Mart Raudsepp from comment #7) > (In reply to Aaron W. Swenson from comment #6) > > I understood what you meant. > > > > I've figured out all the problems. > > > > In addition to the locale.gen changes and a patch to take care of a > > double-free, src_test() needs: > > > > cp common/test-core/unittest_support.py \ > > "${BUILD_DIR}"/common/test-core/ || die; > > cd "${BUILD_DIR}" || die; > > XDG_DATA_HOME="${T}/$(whoami)" emake check > > Odd, doesn't it work with xdg_environment_reset already from pkg_setup that > sets XDG_DATA_HOME to "${HOME}/.local/share" (HOME is in > $PORTAGE_BUILDDIR/homedir)? Is that for some reason ineffective in src_test > or it needs $(whoami) as the subdir name for some odd reason? If that's the > case, I'd try to use ${USER} for now instead of $(whoami), or whatever is > appropriate from bash and the env. I don't know what the test is trying to achieve, but it's checking if gnc_build_userdata_dir (or something like that) is equal to g_tmpdir (or something like that). Naturally, they aren't because gnc_build_userdata_dir (or something like that) will check if XDG_DATA_HOME is defined, or default to something else. g_tmpdir will use the value of TMPDIR if it's defined, or default to something else. The test assumes that neither is defined and sets them to the same place or something. There's some silliness that's going on that I don't grok. I'm going to bring this upstream, too. > > I haven't figured out how to skip particular tests, yet. > > > > Is this acceptable for now? I'm going to raise the missing locale causes > > test failure issue upstream. > > I'd say it's not ideal, but if you state clearly what locales need to be > present for the tests to pass, it ought to be fine for getting it done for > webkit-gtk riddance needs (and stable really being broken as-is). Great. I'll get this done soon. The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=abd6bba4c31bb6e546bb8e1da37fa971ffa55746 commit abd6bba4c31bb6e546bb8e1da37fa971ffa55746 Author: Aaron W. Swenson <titanofold@gentoo.org> AuthorDate: 2018-02-18 12:31:26 +0000 Commit: Aaron W. Swenson <titanofold@gentoo.org> CommitDate: 2018-02-18 12:31:44 +0000 app-office/gnucash: Fix 2.7.4 test failures Fix a double-free bug in test-userdata-dir.c. Copy over common/test-core/unittest_support.py to the BUILDDIR so that it can be sourced by the python-bindings test. Set XDG_DATA_HOME so that it’s pointing to the same place as TMPDIR. Not fixed: Locale dependent tests. /etc/locale.gen needs the following: en_GB.UTF-8 UTF-8 en_US.UTF-8 UTF-8 fr_FR.UTF-8 UTF-8 Bug: https://bugs.gentoo.org/647596 Package-Manager: Portage-2.3.19, Repoman-2.3.6 .../gnucash/files/gnucash-2.7.4-double_free.patch | 12 ++ app-office/gnucash/gnucash-2.7.4-r1.ebuild | 162 +++++++++++++++++++++ 2 files changed, 174 insertions(+)} Created attachment 520072 [details] LastTest.log -r1 > 56 - test-gnc-timezone (Failed) > > 99% tests passed, 1 tests failed out of 98 Created attachment 520078 [details, diff]
skip test-gnc-timezone test for now
If we decide to just disable the timezone test for now, then hopefully this patch works. Completely untested by me.
ping, I still can't last rite security vulnerable stuff; and stable x86 users still can not build current stable gnucash deptree. Please apply the workaround and push keywords, just push keywords or whatnot. We shouldn't be waiting for days here - the stable x86 tree is broken right now. (In reply to Mart Raudsepp from comment #12) > ping, I still can't last rite security vulnerable stuff; and stable x86 > users still can not build current stable gnucash deptree. Please apply the > workaround and push keywords, just push keywords or whatnot. We shouldn't be > waiting for days here - the stable x86 tree is broken right now. Sorry, school and work are both being quite demanding on my time, and solving GnuCash things are nontrivial for me.(In reply to Mart Raudsepp from comment #11) > Created attachment 520078 [details, diff] [details, diff] > skip test-gnc-timezone test for now > > If we decide to just disable the timezone test for now, then hopefully this > patch works. Completely untested by me. This is probably not a good thing to do. There's a section of the test in which the behavior is modified by the value of __LP64__. __LP64__ isn't being set to 1 by GCC when apparently it should be? At any rate, __LP64__ has some usefulness elsewhere in the code for something that has nothing to do with timezones: libgnucash/backend/sql/gnc-sql-column-table-entry.cpp I don't know how important that is, or how to test it. (I only had 20 minutes to spare for now.) Or, maybe the test is wrong and it shouldn't be excluding the LMT zone just because it's on a 32 bit system. Is there an expert on opinions that can give us an opinion that's close enough to expert level? We have 8 errors of > /var/tmp/portage/app-office/gnucash-2.7.4-r1/work/gnucash-2.7.4/libgnucash/engine/test/gtest-gnc-timezone.cpp:162: Failure > Expected: tz->base_utc_offset().total_seconds() > Which is: 27804 > To be equal to: 28800 Which make sense: GnuCash is testing from year 1893 onwards in test_IANA_Perth_tz ( https://github.com/Gnucash/gnucash/blob/master/libgnucash/engine/test/gtest-gnc-timezone.cpp#L145), however, the earliest UNIX timestamp which can be used on a real 32-bit system is "1901-12-13 20:45:52". And 1901-1893 = 8... tests for year 1902 until 2048 seems to pass. Same problem in test_IANA_Minsk_tz. For testing I applied > --- a/libgnucash/engine/test/gtest-gnc-timezone.cpp > +++ b/libgnucash/engine/test/gtest-gnc-timezone.cpp > @@ -142,7 +142,7 @@ TEST(gnc_timezone_constructors, test_IANA_Belize_tz) > TEST(gnc_timezone_constructors, test_IANA_Perth_tz) > { > TimeZoneProvider tzp("Australia/Perth"); > - for (int year = 1893; year < 2048; ++year) > + for (int year = 1902; year < 2048; ++year) > { > auto tz = tzp.get(year); > #ifdef __LP64__ > @@ -204,7 +204,7 @@ TEST(gnc_timezone_constructors, test_IANA_Perth_tz) > TEST(gnc_timezone_constructors, test_IANA_Minsk_tz) > { > TimeZoneProvider tzp("Europe/Minsk"); > - for (int year = 1870; year < 2020; ++year) > + for (int year = 1902; year < 2020; ++year) > { > auto tz = tzp.get(year); > #ifdef __LP64__ > and tests passed. My suggestion: Apply this patch/skip this test only for x86 and report upstream. Once upstream has adjusted the test to respect 32-bit capabilities, we can re-enable. Created attachment 520520 [details, diff]
gnucash-x86-tests-fix.patch
Please proceed Whissi as you see fit; we need to get this stabled finally :) Leave this bug open (Bug: not Closes: tag) to track proper upstream fix for timezones and the locale workaround. The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=432f3f5e66e510ae29429064071ce83465ad6bb5 commit 432f3f5e66e510ae29429064071ce83465ad6bb5 Author: Thomas Deutschmann <whissi@gentoo.org> AuthorDate: 2018-02-22 11:44:00 +0000 Commit: Thomas Deutschmann <whissi@gentoo.org> CommitDate: 2018-02-22 11:44:00 +0000 app-office/gnucash: Add patch to address test failures on 32-bit platforms Bug: https://bugs.gentoo.org/647596 Package-Manager: Portage-2.3.24, Repoman-2.3.6 ...ucash-2.7.4-fix-tests-for-32bit-platforms.patch | 56 ++++++++++++++++++++++ app-office/gnucash/gnucash-2.7.4-r1.ebuild | 5 +- 2 files changed, 60 insertions(+), 1 deletion(-)} Any updates on this? Did it get an issue made as requested in the PR? Can the URL be changed to that, if so? Upstream closed my PR. Issue with LP64 should be brought to GnuCash's dedicated bug tracker. As I don't have an account for GnuCash's bug tracker and I don't use that program I am asking the maintainer to take care of that. Thanks. This has been fixed upstream, and should have been considered fixed when we stabled 2.7.4-r1 with the patches. https://github.com/Gnucash/gnucash/commit/5520fae838bfddc92385c85706e741a753458c5d#diff-a08895af7b8368e8ea409efc29e1e64e |