Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Some failing cases and test notes #1

Closed
aquilax opened this issue Jul 24, 2020 · 7 comments
Closed

Some failing cases and test notes #1

aquilax opened this issue Jul 24, 2020 · 7 comments

Comments

@aquilax
Copy link

aquilax commented Jul 24, 2020

Hey, just wanted to share my notes from quick testing the tool.
I ran it against one of my ledger files since the README states that acc uses the ledger format.

$ RUST_BACKTRACE=full acc --file bgn.ledger bal
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40
stack backtrace:
   0:     0x5563c08cbdf4 - backtrace::backtrace::libunwind::trace::he25250f78ba1020d
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1:     0x5563c08cbdf4 - backtrace::backtrace::trace_unsynchronized::hd7e25f35da233a1e
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2:     0x5563c08cbdf4 - std::sys_common::backtrace::_print_fmt::h5578a566a4c2ae4e
                               at src/libstd/sys_common/backtrace.rs:84
   3:     0x5563c08cbdf4 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hd4f87054ec86e936
                               at src/libstd/sys_common/backtrace.rs:61
   4:     0x5563c08e902c - core::fmt::write::h393d55821fa8e2cb
                               at src/libcore/fmt/mod.rs:1025
   5:     0x5563c08ca3b7 - std::io::Write::write_fmt::hc6645302c23e4504
                               at src/libstd/io/mod.rs:1426
   6:     0x5563c08cdd5e - std::sys_common::backtrace::_print::he8a0a3ebec70e1d6
                               at src/libstd/sys_common/backtrace.rs:65
   7:     0x5563c08cdd5e - std::sys_common::backtrace::print::hc2635fd6c7743ade
                               at src/libstd/sys_common/backtrace.rs:50
   8:     0x5563c08cdd5e - std::panicking::default_hook::{{closure}}::h9849388798b72b2b
                               at src/libstd/panicking.rs:193
   9:     0x5563c08cda51 - std::panicking::default_hook::h66e3afff11b02e47
                               at src/libstd/panicking.rs:210
  10:     0x5563c08ce38b - std::panicking::rust_panic_with_hook::hf6d05969fcffff03
                               at src/libstd/panicking.rs:471
  11:     0x5563c08cdf3e - rust_begin_unwind
                               at src/libstd/panicking.rs:375
  12:     0x5563c08e611e - core::panicking::panic_fmt::he794217b0df3f806
                               at src/libcore/panicking.rs:84
  13:     0x5563c08e606a - core::panicking::panic::h104b3bcaeb5f1ff0
                               at src/libcore/panicking.rs:51
  14:     0x5563c08bf12e - acc::balancer::balance_transactions::he39be7a18fdbf8ff
  15:     0x5563c08ab45e - acc::main::h6ed75259d58e00ee
  16:     0x5563c08a7f63 - std::rt::lang_start::{{closure}}::haf197709f72449ec
  17:     0x5563c08cde23 - std::rt::lang_start_internal::{{closure}}::h4aedff5d9863b36b
                               at src/libstd/rt.rs:52
  18:     0x5563c08cde23 - std::panicking::try::do_call::h0630fe3da32a089b
                               at src/libstd/panicking.rs:292
  19:     0x5563c08cfe6a - __rust_maybe_catch_panic
                               at src/libpanic_unwind/lib.rs:78
  20:     0x5563c08ce820 - std::panicking::try::hdfbe2fd873ac646c
                               at src/libstd/panicking.rs:270
  21:     0x5563c08ce820 - std::panic::catch_unwind::hd7a9c10fa2bee1bc
                               at src/libstd/panic.rs:394
  22:     0x5563c08ce820 - std::rt::lang_start_internal::hb8081e0e7e42ac0d
                               at src/libstd/rt.rs:51
  23:     0x5563c08acfa2 - main
  24:     0x7f11aee2c0b3 - __libc_start_main
  25:     0x5563c089e17e - _start
  26:                0x0 - <unknown>

Here are some failing test cases:

No currency

2011-04-10 Test
  Test:Test  1900.00
  Test

Two trailing spaces on the last line

2011-04-10 * Test
  Test:Test  $1900.00
  Test:Test

Suffix currency

2011-04-10 * Test
  Test:Test  1900.00 SEK
  Test:Test

Single space between account and currency:

2011-04-10 * Test
  Test:Test $1900.00
  Test:Test

The report looks a bit strange in this case:

$ cat bgn.ledger 
2011-04-10 * Test
  Test:Test  # 1900.00
  Test

2011-04-10 * Test
  Test:Test  $ 1900.00
  Test
$ acc --file bgn.ledger bal
   # 0.00 
   $ 0.00 Test
# 1900.00 
$ 1900.00   Test
---------
        0 
$ acc --file bgn.ledger reg
2011-04-10 * Test    Test:Test    #  1900.002       # 1900.00
                     Test         # -1900.002       #    0.00
2011-04-10 * Test    Test:Test    $  1900.002       #    0.00
                                                   $ 1900.00
                     Test         $ -1900.002       #    0.00
                                                   $    0.00

Cheers!
@tbm
Copy link

tbm commented Jul 25, 2020

Single space between account and currency

Not the maintainer, just a random guy, but this is not valid in ledger. It has to be 2 spaces or one tab.

@Shadow53
Copy link

Can confirm the above: account names can have single spaces in them, so a single space is not a sufficient separator.

@aquilax
Copy link
Author

aquilax commented Jul 25, 2020

Hi. Yes, you're both right that single space s not a valid row.
I didn't make myself clear that in this case the app throws:

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40

@rudolfschmidt
Copy link
Owner

Thank you very much for your time and effort to report issues.

See comments below

Hey, just wanted to share my notes from quick testing the tool.
I ran it against one of my ledger files since the README states that acc uses the ledger format.

* No support for comments yet https://www.ledger-cli.org/3.0/doc/ledger3.html#Commenting-on-your-Journal

I am not sure which version you have tested because I commit almost daily, but please test it again, because comments should work, at least now.

* No support for slash separated dates

This is true, I will add support for that soon. I add this to the Todo section

* Nu support for expressions (but noticed that is already on the ToDo list)

Yes, one of the major reasons I have started acc. Ledger is not really maintained anymore and hledger does not want to add support for expressions

* I got this error when I used values without currency

I fixed that issue. I have forgotten the use-case when you do not add a commodity/currency. In the previous version, you had to use a commodity and amount or leave both empty. Now commodity is optional and an empty posting is empty when it has no amount.

$ RUST_BACKTRACE=full acc --file bgn.ledger bal
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40
stack backtrace:
   0:     0x5563c08cbdf4 - backtrace::backtrace::libunwind::trace::he25250f78ba1020d
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1:     0x5563c08cbdf4 - backtrace::backtrace::trace_unsynchronized::hd7e25f35da233a1e
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2:     0x5563c08cbdf4 - std::sys_common::backtrace::_print_fmt::h5578a566a4c2ae4e
                               at src/libstd/sys_common/backtrace.rs:84
   3:     0x5563c08cbdf4 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hd4f87054ec86e936
                               at src/libstd/sys_common/backtrace.rs:61
   4:     0x5563c08e902c - core::fmt::write::h393d55821fa8e2cb
                               at src/libcore/fmt/mod.rs:1025
   5:     0x5563c08ca3b7 - std::io::Write::write_fmt::hc6645302c23e4504
                               at src/libstd/io/mod.rs:1426
   6:     0x5563c08cdd5e - std::sys_common::backtrace::_print::he8a0a3ebec70e1d6
                               at src/libstd/sys_common/backtrace.rs:65
   7:     0x5563c08cdd5e - std::sys_common::backtrace::print::hc2635fd6c7743ade
                               at src/libstd/sys_common/backtrace.rs:50
   8:     0x5563c08cdd5e - std::panicking::default_hook::{{closure}}::h9849388798b72b2b
                               at src/libstd/panicking.rs:193
   9:     0x5563c08cda51 - std::panicking::default_hook::h66e3afff11b02e47
                               at src/libstd/panicking.rs:210
  10:     0x5563c08ce38b - std::panicking::rust_panic_with_hook::hf6d05969fcffff03
                               at src/libstd/panicking.rs:471
  11:     0x5563c08cdf3e - rust_begin_unwind
                               at src/libstd/panicking.rs:375
  12:     0x5563c08e611e - core::panicking::panic_fmt::he794217b0df3f806
                               at src/libcore/panicking.rs:84
  13:     0x5563c08e606a - core::panicking::panic::h104b3bcaeb5f1ff0
                               at src/libcore/panicking.rs:51
  14:     0x5563c08bf12e - acc::balancer::balance_transactions::he39be7a18fdbf8ff
  15:     0x5563c08ab45e - acc::main::h6ed75259d58e00ee
  16:     0x5563c08a7f63 - std::rt::lang_start::{{closure}}::haf197709f72449ec
  17:     0x5563c08cde23 - std::rt::lang_start_internal::{{closure}}::h4aedff5d9863b36b
                               at src/libstd/rt.rs:52
  18:     0x5563c08cde23 - std::panicking::try::do_call::h0630fe3da32a089b
                               at src/libstd/panicking.rs:292
  19:     0x5563c08cfe6a - __rust_maybe_catch_panic
                               at src/libpanic_unwind/lib.rs:78
  20:     0x5563c08ce820 - std::panicking::try::hdfbe2fd873ac646c
                               at src/libstd/panicking.rs:270
  21:     0x5563c08ce820 - std::panic::catch_unwind::hd7a9c10fa2bee1bc
                               at src/libstd/panic.rs:394
  22:     0x5563c08ce820 - std::rt::lang_start_internal::hb8081e0e7e42ac0d
                               at src/libstd/rt.rs:51
  23:     0x5563c08acfa2 - main
  24:     0x7f11aee2c0b3 - __libc_start_main
  25:     0x5563c089e17e - _start
  26:                0x0 - <unknown>

Thanks for the report. I have added an error message to cover situations when a line is not recognized.

Here are some failing test cases:

No currency

Fixed

2011-04-10 Test
  Test:Test  1900.00
  Test

Two trailing spaces on the last line

Fixed

2011-04-10 * Test
  Test:Test  $1900.00
  Test:Test

Suffix currency

I will add support for that soon. Added to Todo section

2011-04-10 * Test
  Test:Test  1900.00 SEK
  Test:Test

Single space between account and currency:

Like tbm and Shadow53 mentioned, its not a valid ledger format. What should happen when someone wants to name an account "Test $1900.00"? Should this be allowed or not? But I see later that this was the argument for you.

2011-04-10 * Test
  Test:Test $1900.00
  Test:Test

The report looks a bit strange in this case:

The reason for that is an issue with a library that I use internally. See rust-num/num-rational#10

You see in the report that it formats decimals wrong. Its the first time I have to notice it. I need to do some research if nothing else works I will replace the library with something better or develop my own version since it just converts numbers into rationals to calculate them probably (avoiding rounding errors) and print them, something I can write myself if needed.

$ cat bgn.ledger 
2011-04-10 * Test
  Test:Test  # 1900.00
  Test

2011-04-10 * Test
  Test:Test  $ 1900.00
  Test
$ acc --file bgn.ledger bal
   # 0.00 
   $ 0.00 Test
# 1900.00 
$ 1900.00   Test
---------
        0 
$ acc --file bgn.ledger reg
2011-04-10 * Test    Test:Test    #  1900.002       # 1900.00
                     Test         # -1900.002       #    0.00
2011-04-10 * Test    Test:Test    $  1900.002       #    0.00
                                                   $ 1900.00
                     Test         $ -1900.002       #    0.00
                                                   $    0.00

Cheers!

Single space between account and currency

Not the maintainer, just a random guy, but this is not valid in ledger. It has to be 2 spaces or one tab.
I would not say "random guy" since you are involved in the ledger project for a long time :)

Hi. Yes, you're both right that single space s not a valid row.
I didn't make myself clear that in this case the app throws:

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/f3e1a954d2ead4e2fc197c7da7d71e6c61bad196/src/libcore/macros/mod.rs:15:40

I hope it works not. Please test it again, I did and I get an Error

While parsing file "test.ledger" at line 3:
Only one posting with null amount allowed per transaction

because he recognizes it as two empty posts.

@aquilax
Copy link
Author

aquilax commented Jul 26, 2020

Hey @rudolfschmidt Thank you for taking a look at the report. I'm not sure what you mean by "Ledger is not really maintained anymore". Last commit was 5 days ago and last release was in May, but I am happy to have more options.
Cheers!

@rudolfschmidt
Copy link
Owner

rudolfschmidt commented Jul 27, 2020 •
edited
Loading

Hey @rudolfschmidt Thank you for taking a look at the report. I'm not sure what you mean by "Ledger is not really maintained anymore". Last commit was 5 days ago and last release was in May, but I am happy to have more options.
Cheers!

you are welcome, the main maintainer john w. said that he is not interested to develop ledger anymore, so the project is not really in development anymore.

@rudolfschmidt
Copy link
Owner

All the mentioned bugs should be fixed. Please open a new issue if your bugs still exist (in dev version).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@aquilax @rudolfschmidt @tbm @Shadow53