Re: Reports: Utilization of urls
On Thursday 10 March 2016 18:58:56 Carsten Rinke wrote: > I have the same point of view regarding the categorization: > This is adding an optional representation mode to existing reports. > Not making up new reports. > This option is available already for the networth line chart (even > though differently implemented), so I rather see this a bug fix > instead of a new feature. > It's not a bug fix. The line chart views weren't there for these reports so you're not fixing something that didn't work properly. You're adding functionality that simply wasn't there yet. There's no use in trying to debate that. Whether the new feature should be eligible for maint inclusion can be debatable. You'll find my motivation for not including it below. > At the same time I see this a minor modification. So waiting 2 years > to make this available for other users that is a long way. > But it has happened before that I have underestimated the complexity > of the issue > It's not so much you underestimating the complexity of the issue. I agree that the change itself in this case looks relatively small. The complexity comes from the state of the guile code as a whole in gnucash. There is insufficient isolation in many cases. As a result making a trivial change for one issue can easily break another part of the code without any of us realizing. That's why I tend to be extremely conservative in making changes in guile code in the stable series. I have underestimated these inter dependencies more than once in the past (among others with several of your patches I did review and commit only to revert them afterwards again due to complications). In each of the cases I believed the change was sufficiently local to be ok and was wrong. I don't feel like repeating that exercise on a regular basis. If others want to review your patches and estimate whether they are safe to include in maint, I'm fine with that. I will stick to my conservative selection of only applying patches to maint I have sufficient confidence in they won't break the code in unexpected ways. Note I would feel much more confident if we had a good unit test set available for most of our guile modules, which we don't. If you feel like it that's a very good area to contribute in as well :) Regards, Geert ___ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Unit tests
Hi, While updating the import-map tests to reflect the changes to use guid's, I came across the following, just wondering if it is my set-up. Firstly I had to change configure.ac at line 850 and 856 to add a directory src to the paths [AC_CHECK_FILES([/usr/src/gmock/gmock-all.cc to [AC_CHECK_FILES([/usr/src/gmock/src/gmock-all.cc /usr/src/gtest/gtest-main.cc to/usr/src/gtest/src/gtest-main.cc test-kvp-frame.cpp line 153, comparison between signed and unsigned integer expressions EXPECT_EQ (keys.size (), 1); to EXPECT_EQ ((int)keys.size (), 1); these changes allowed me to run the tests and update the engine import-map tests but they fail later on at, all seem to be the same error PASS: test-vendor FAIL: test-test-extras FAIL: test-account FAIL: test-split PASS: test-lots the log for test-test-extras is below... Backtrace: In unknown file: ?: 0* [primitive-load "./test-test-extras.scm"] In ./test-test-extras.scm: 23: 1* (use-modules (gnucash engine test test-extras)) 23: 2 (eval-case (# # *unspecified*) (else #)) 23: 3 (begin (process-use-modules (list (list #))) *unspecified*) In unknown file: ?: 4* [process-use-modules (((gnucash engine test test-extras)))] ?: 5 (let* ((interfaces #)) (call-with-deferred-observers (lambda () #))) ?: 6* [map # (((gnucash engine test test-extras)))] ?: 7* [# ((gnucash engine test test-extras))] ?: 8* (or (apply resolve-interface mif-args) (error "no such module" mif-args)) ?: 9* [apply # (#)] ?: 10 [resolve-interface (gnucash engine test test-extras)] ... ?: 11 (let* (# # # # ...) (and # #) (if # public-i #)) ?: 12* (and (or (not module) (not public-i)) (error "no code for module" name)) ?: 13 [error "no code for module" (gnucash engine test test-extras)] ... ?: 14 [scm-error misc-error #f ...] : In procedure scm-error in expression (scm-error (quote misc-error) #f ...): : no code for module (gnucash engine test test-extras) FAIL test-test-extras (exit status: 1) Not sure what that means. Robert ___ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Re: Unit tests
> On Mar 13, 2016, at 7:33 AM, Robert Fewell <14ubo...@gmail.com> wrote: > > Hi, > > While updating the import-map tests to reflect the changes to use guid's, I > came across the following, just wondering if it is my set-up. > > Firstly I had to change configure.ac at line 850 and 856 to add a directory > src to the paths > > [AC_CHECK_FILES([/usr/src/gmock/gmock-all.cc > > to [AC_CHECK_FILES([/usr/src/gmock/src/gmock-all.cc > > /usr/src/gtest/gtest-main.cc > > to/usr/src/gtest/src/gtest-main.cc > Don't do that. Configure is set up to find the path on Debian/Ubuntu and Fedora. Users of other distros should use the --with-gtest* and --with-gmock* configure arguments to set the paths. > > test-kvp-frame.cpp line 153, comparison between signed and unsigned integer > expressions > > EXPECT_EQ (keys.size (), 1); > > to EXPECT_EQ ((int)keys.size (), 1); Don't use C-style casts in C++, and anyway it's better to make the constant unsigned: EXPECT_EQ (keys.size), 1U); > > these changes allowed me to run the tests and update the engine import-map > tests but they fail later on at, all seem to be the same error > > PASS: test-vendor > FAIL: test-test-extras > FAIL: test-account > FAIL: test-split > PASS: test-lots > > the log for test-test-extras is below... > > Backtrace: > In unknown file: > ?: 0* [primitive-load "./test-test-extras.scm"] > In ./test-test-extras.scm: > 23: 1* (use-modules (gnucash engine test test-extras)) > 23: 2 (eval-case (# # *unspecified*) (else #)) > 23: 3 (begin (process-use-modules (list (list #))) *unspecified*) > In unknown file: > ?: 4* [process-use-modules (((gnucash engine test test-extras)))] > ?: 5 (let* ((interfaces #)) (call-with-deferred-observers (lambda () > #))) > ?: 6* [map # (((gnucash engine test > test-extras)))] > ?: 7* [# ((gnucash engine test test-extras))] > ?: 8* (or (apply resolve-interface mif-args) (error "no such module" > mif-args)) > ?: 9* [apply # (#)] > ?: 10 [resolve-interface (gnucash engine test test-extras)] >... > ?: 11 (let* (# # # # ...) (and # #) (if # public-i #)) > ?: 12* (and (or (not module) (not public-i)) (error "no code for module" > name)) > ?: 13 [error "no code for module" (gnucash engine test test-extras)] >... > ?: 14 [scm-error misc-error #f ...] > > : In procedure scm-error in expression (scm-error (quote > misc-error) #f ...): > : no code for module (gnucash engine test test-extras) > FAIL test-test-extras (exit status: 1) That's a scheme stack trace. It's telling you that ltdl couldn't find a library. If ~/.cache/guile/ccache exists, nuke it and try again. Regards, John Ralls ___ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Re: Reports: Utilization of urls
Hi Geert, "it is not a bug fix" From end user point of view I disagree, but I agree that this is not an item to waste energy on. "several of your patches I did review and commit only to revert them afterwards again" I think that references the - cashflow calculation issue, which the patch was later-on declared "obsolete - not a bug, but misunderstanding of function" - the overlapping x-axis label issue, that showed problems on other distributions, which I could not see in my environment "unit test set available for most of our guile modules" Even though I very much support this idea: I don't think that this would have avoided the problems above, especially as it is not feasible for me to run a mutli-distro environment. For me the question is, if there is man-power available to qualify and verify the solutions and patches before they are committed, best case: in different environments. My perception is that currently this all falls back onto only your table. Is my perception correct? Independent of that: What do you have in mind with a "unit test set"? You mean documentation as kind of work instructions for testing? Or do you mean an automated test framework? Regards, Carsten On 13.03.2016 14:39, Geert Janssens wrote: On Thursday 10 March 2016 18:58:56 Carsten Rinke wrote: > I have the same point of view regarding the categorization: > This is adding an optional representation mode to existing reports. > Not making up new reports. > This option is available already for the networth line chart (even > though differently implemented), so I rather see this a bug fix > instead of a new feature. > It's not a bug fix. The line chart views weren't there for these reports so you're not fixing something that didn't work properly. You're adding functionality that simply wasn't there yet. There's no use in trying to debate that. Whether the new feature should be eligible for maint inclusion can be debatable. You'll find my motivation for not including it below. > At the same time I see this a minor modification. So waiting 2 years > to make this available for other users that is a long way. > But it has happened before that I have underestimated the complexity > of the issue > It's not so much you underestimating the complexity of the issue. I agree that the change itself in this case looks relatively small. The complexity comes from the state of the guile code as a whole in gnucash. There is insufficient isolation in many cases. As a result making a trivial change for one issue can easily break another part of the code without any of us realizing. That's why I tend to be extremely conservative in making changes in guile code in the stable series. I have underestimated these inter dependencies more than once in the past (among others with several of your patches I did review and commit only to revert them afterwards again due to complications). In each of the cases I believed the change was sufficiently local to be ok and was wrong. I don't feel like repeating that exercise on a regular basis. If others want to review your patches and estimate whether they are safe to include in maint, I'm fine with that. I will stick to my conservative selection of only applying patches to maint I have sufficient confidence in they won't break the code in unexpected ways. Note I would feel much more confident if we had a good unit test set available for most of our guile modules, which we don't. If you feel like it that's a very good area to contribute in as well :) Regards, Geert ___ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Re: Reports: Utilization of urls
On Sunday 13 March 2016 18:49:33 Carsten Rinke wrote: > "several of your patches I did review and commit only to revert them > afterwards again" > I think that references the > - cashflow calculation issue, which the patch was later-on declared > "obsolete - not a bug, but misunderstanding of function" > - the overlapping x-axis label issue, that showed problems on other > distributions, which I could not see in my environment > Those in addition to commits unrelated to your work I did myself at some point and had to revert... :( > "unit test set available for most of our guile modules" > Even though I very much support this idea: I don't think that this > would have avoided the problems above, especially as it is not > feasible for me to run a mutli-distro environment. The overlapping x-axis regression was reported in https://bugzilla.gnome.org/show_bug.cgi?id=737815 There is no mention of it being platform specific. On the contrary it was reported on several platforms, being Windows, OS X and several flavors of linux. I don't expect you to run a multi-distro environment and there's no need to. Having said that, you are probably right that this particular issue would not have been caught with unit tests, because it only had visual repercussions resulting from a bug on jqplot or wrong use of that library. There was no data to match against. > For me the question is, if there is man-power available to qualify and > verify the solutions and patches before they are committed, best > case: in different environments. > My perception is that currently this all falls back onto only your > table. Is my perception correct? I do most of the reviews of scheme code indeed, although John does so as well from time to time. That gives us 2 platforms already if you like (OS X and Fedora linux). > > Independent of that: > What do you have in mind with a "unit test set"? Unit tests are there to verify that isolated functions return expected results on given input. So a "unit test" set would be a series of tests that do this for all functions written in guile. That's the theory. We won't be able to do that for *all* guile code, but we could do so for many. > You mean documentation as kind of work instructions for testing? > Or do you mean an automated test framework? I mean a set of test routines that are called whenever you run make check on our source tree. There are already a couple in place, like for example in src/report/standard-reports/test But our test coverage a (far) from complete. Regards, Geert ___ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
