Bug 685102 - Scheduled Transactions don't always respect weekends for… #671

jeanlaroche · 2020-03-22T06:44:58Z

… first occurrence
I gave it another shot. This solution is, I think, simple and acceptable. It does duplicate a small portion of code, but I don't think that's objectionable.
I initially thought the start date in the Recurrence structure could be adjusted if it fell on a week end (and the user does not want that), but if you adjust the start date, subsequent occurrences will happen on the wrong date because they're computed based on the moved start date.
BTW the following tests fail on my machine:

Showing Recent Messages
The following tests FAILED:
	  1 - test-exp-parser (Failed)
	  2 - test-link-module-app-utils (Failed)
	  3 - test-print-parse-amount (Not Run)
	  4 - test-scm-query-string (Failed)
	  5 - test-sx (Not Run)
	 10 - test-app-utils (Failed)
	 11 - test-dom-converters1 (Not Run)
	 12 - test-kvp-frames (Not Run)
	 13 - test-load-backend (Not Run)
	 14 - test-load-xml2 (Failed)
	 15 - test-load-example-account (Failed)
	 16 - test-string-converters (Not Run)
	 17 - test-xml-account (Not Run)
	 18 - test-xml-commodity (Not Run)
	 19 - test-xml-pricedb (Not Run)
	 20 - test-xml-transaction (Not Run)
	 21 - test-xml2-is-file (Failed)
	 22 - test-real-data (Failed)
	 23 - test-backend-dbi (Not Run)
	 24 - test-column-types (Not Run)
	 25 - test-sqlbe (Not Run)
	 26 - test-gnc-glib-utils (Not Run)
	 27 - test-resolve-file-path (Not Run)
	 28 - test-userdata-dir (Not Run)
	 29 - test-gnc-path-util (Failed)
	 30 - test-link (Not Run)
	 31 - test-load-engine (Not Run)
	 32 - test-guid (Not Run)
	 33 - test-object (Not Run)
	 34 - test-commodities (Not Run)
	 35 - test-qof (Not Run)
	 36 - test-engine (Not Run)
	 37 - test-account-object (Not Run)
	 38 - test-group-vs-book (Not Run)
	 39 - test-lots (Not Run)
	 40 - test-querynew (Not Run)
	 41 - test-query (Not Run)
	 42 - test-split-vs-account (Not Run)
	 43 - test-transaction-reversal (Not Run)
	 44 - test-transaction-voiding (Not Run)
	 45 - test-recurrence (Not Run)
	 46 - test-business (Not Run)
	 47 - test-address (Not Run)
	 48 - test-customer (Not Run)
	 49 - test-employee (Not Run)
	 50 - test-job (Not Run)
	 51 - test-vendor (Not Run)
	 52 - test-numeric (Not Run)
	 53 - test-gnc-guid (Not Run)
	 54 - test-kvp-value (Not Run)
	 55 - test-qofsession (Not Run)
	 56 - test-gnc-int128 (Not Run)
	 57 - test-gnc-rational (Not Run)
	 58 - test-gnc-numeric (Not Run)
	 59 - test-gnc-timezone (Not Run)
	 60 - test-gnc-datetime (Not Run)
	 61 - test-import-map (Not Run)
	 62 - test-qofquerycore (Not Run)
	 63 - test-scm-query (Failed)
	 68 - test-load-c (Failed)
	 69 - test-modsysver (Failed)
	 70 - test-incompatdep (Failed)
	 71 - test-agedver (Failed)
	 72 - test-dynload (Failed)
	 81 - test-link-module-tax-us (Not Run)
	 82 - test-link-module-gnome-utils (Failed)
	 84 - test-import-parse (Failed)
	 85 - test-link-generic-import (Not Run)
	 86 - test-import-pending-matches (Not Run)
	 87 - test-import-account-matcher (Not Run)
	 88 - test-aqb (Failed)
	 89 - test-tokenizer (Failed)
	 90 - test-link-ofx (Not Run)
	 91 - test-link-qif-imp (Not Run)
	 95 - test-link-module-ledger-core (Not Run)
	 96 - test-link-module-register-core (Not Run)
	 97 - test-link-module-register-gnome (Not Run)
	101 - test-link-module-report-locale-specific-us (Not Run)
	102 - test-link-module-report-gnome (Failed)
	104 - test-link-module-report-system (Failed)
Errors while running CTest
make: *** [RUN_TESTS_buildpart_0] Error 8
Command /bin/sh failed with exit code 2

christopherlam · 2020-03-22T08:33:51Z

46/133 Test 46: test-recurrence ..............................***Failed 5.01 sec
FAILURE dom don't match /gnucash/libgnucash/engine/test/test-recurrence.c:103

You'll need to dig into test-recurrence and explore why the test at line 103 has failed.

jralls · 2020-03-22T18:33:32Z

One must wonder why the author of that test thought it useful to test 24,300,000 random recurrences, not to stop on the first failure, and not to print out the parameters that cause the failure.

Anyway, test-recurrence fails because the day of the month (dom) differs for subsequent recurrences and the start date is < 28. The outer-most loop of the test checks every value of weekend_adjust and the fact that the test passed should have been evidence that the feature wasn't working. Oops.

I think changing line 98 to

else if (wadj == WEEKEND_ADJ_NONE)

should get the test to pass. Extra credit for instead having switch(wadj) with working tests for each alternative.

@jeanlaroche did you run the check or the test target? The former builds the dependencies for the tests, the latter just tries to execute them without building anything. That works fine once check has run once, but not before.

jeanlaroche · 2020-03-22T18:44:29Z

I think changing line 98 to
else if (wadj == WEEKEND_ADJ_NONE)
should get the test to pass. Extra credit for instead having switch(wadj) with working tests for each alternative.

I'm modifying the test so it now handles wadj. I should also make it fail at the first error.

@jeanlaroche did you run the check or the test target? The former builds the dependencies for the tests, the latter just tries to execute them without building anything. That works fine once check has run once, but not before.

Ah! That's what I've been missing. Thanks. I'm currently using the test-recurrence target and that's working great for fixing it, but not for running all tests obviously.

jralls · 2020-03-22T18:46:19Z

A much better approach.

It does duplicate a small portion of code, but I don't think that's objectionable.

But it would be no more work to extract that code to a helper function, and it would help shorten recurrenceNextInstance.

… first occurrence

jeanlaroche · 2020-03-22T19:45:47Z

I'm still unable to run the full set of tests on my machine, but test-recurrence is now fixed (and improved).

jralls · 2020-03-22T19:53:16Z

What's failing make check?

jeanlaroche · 2020-03-22T19:59:20Z

What's failing make check?

Showing Recent Messages
Ld /Users/Shari/gnucash-stable/build_xcode/bin/test-gnc-path-util normal x86_64
    cd /Users/Shari/gnucash-stable/src/gnucash-git
    export MACOSX_DEPLOYMENT_TARGET=10.14
    /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -L/Users/Shari/gnucash-stable/build_xcode/bin -F/Users/Shari/gnucash-stable/build_xcode/bin -filelist /Users/Shari/gnucash-stable/build_xcode/libgnucash/core-utils/test/gnucash.build/Debug/test-gnc-path-util.build/Objects-normal/x86_64/test-gnc-path-util.LinkFileList -mmacosx-version-min=10.14 -Xlinker -object_path_lto -Xlinker /Users/Shari/gnucash-stable/build_xcode/libgnucash/core-utils/test/gnucash.build/Debug/test-gnc-path-util.build/Objects-normal/x86_64/test-gnc-path-util_lto.o -Xlinker -no_deduplicate -L/usr/local/opt/openblas/lib -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/Users/Shari/gnucash-stable/lib -lintl -lglib-2.0 /Users/Shari/gnucash-stable/lib/libboost_date_time.dylib /Users/Shari/gnucash-stable/lib/libboost_regex.dylib /Users/Shari/gnucash-stable/lib/libboost_locale.dylib /Users/Shari/gnucash-stable/lib/libboost_filesystem.dylib /Users/Shari/gnucash-stable/lib/libboost_system.dylib -L/Users/Shari/gnucash-stable/lib -lgthread-2.0 -lintl -lglib-2.0 /Users/Shari/gnucash-stable/build_xcode/common/test-core/libgtest.a /Users/Shari/gnucash-stable/lib/libboost_date_time.dylib /Users/Shari/gnucash-stable/lib/libboost_regex.dylib /Users/Shari/gnucash-stable/lib/libboost_locale.dylib /Users/Shari/gnucash-stable/lib/libboost_filesystem.dylib /Users/Shari/gnucash-stable/lib/libboost_system.dylib -lgthread-2.0 /Users/Shari/gnucash-stable/build_xcode/common/test-core/libgtest.a -Xlinker -dependency_info -Xlinker /Users/Shari/gnucash-stable/build_xcode/libgnucash/core-utils/test/gnucash.build/Debug/test-gnc-path-util.build/Objects-normal/x86_64/test-gnc-path-util_dependency_info.dat -o /Users/Shari/gnucash-stable/build_xcode/bin/test-gnc-path-util

clang: error: no such file or directory: '/Users/Shari/gnucash-stable/build_xcode/common/test-core/libgtest.a'
clang: error: no such file or directory: '/Users/Shari/gnucash-stable/build_xcode/common/test-core/libgtest.a'

jralls · 2020-03-22T20:13:24Z

That's googletest. The Xcode generator must not be making the connection between that output file and the gtest target. Does Xcode show a gtest target, and if so does building it first get you further along building the tests?

jralls · 2020-03-22T20:28:20Z

Clever touch using the bitwise equals operators to accumulate test results. Thanks.

We can keep using this PR discussion to figure out how to get tests built with Xcode if you like, otherwise gnucash-devel or IRC.

jean added 3 commits March 22, 2020 12:34

Bug 685102 - Scheduled Transactions don't always respect weekends for…

7a61eca

… first occurrence

Make test-recurrence handle wadj

268695f

Move the weekend adjust code in its own function

66fce05

jeanlaroche force-pushed the 685402_schedule_trans_error branch from 5624d12 to 66fce05 Compare March 22, 2020 19:44

code-gnucash-org merged commit 2bbf5b2 into Gnucash:maint Mar 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 685102 - Scheduled Transactions don't always respect weekends for… #671

Bug 685102 - Scheduled Transactions don't always respect weekends for… #671

jeanlaroche commented Mar 22, 2020

christopherlam commented Mar 22, 2020 •

edited

jralls commented Mar 22, 2020

jeanlaroche commented Mar 22, 2020

jralls commented Mar 22, 2020

jeanlaroche commented Mar 22, 2020

jralls commented Mar 22, 2020

jeanlaroche commented Mar 22, 2020

jralls commented Mar 22, 2020

jralls commented Mar 22, 2020

Bug 685102 - Scheduled Transactions don't always respect weekends for… #671

Bug 685102 - Scheduled Transactions don't always respect weekends for… #671

Conversation

jeanlaroche commented Mar 22, 2020

christopherlam commented Mar 22, 2020 • edited

jralls commented Mar 22, 2020

jeanlaroche commented Mar 22, 2020

jralls commented Mar 22, 2020

jeanlaroche commented Mar 22, 2020

jralls commented Mar 22, 2020

jeanlaroche commented Mar 22, 2020

jralls commented Mar 22, 2020

jralls commented Mar 22, 2020

christopherlam commented Mar 22, 2020 •

edited