-
Notifications
You must be signed in to change notification settings - Fork 27
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.
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
Transaction with a single posting should be an input error #130 #132
Conversation
@@ -225,6 +225,8 @@ object Processor { | |||
|
|||
if (postsNoAmount.length > 1) { | |||
throw new InputPosError(s"More than one account posted with unspecified amount: ${postsNoAmount.map(_.accName).mkString(",")}", tx.pos) | |||
} else if (postsWithAmount.isEmpty) { | |||
throw new InputPosError(s"Transaction with single posting: ${tx.posts.head.accName}", tx.pos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the check you added (postsWithAmount.isEmpty
). It is more general than I had imagined.
However, the error message seems to be too narrow. How about: "Transaction with unspecified amount"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will do :)
ignoreDirSuiteTestCases(errorRoot, Glob("bugsInputError/**.exec")) { args: Array[String] => | ||
assertThrows[SettingsError] { | ||
runDirSuiteTestCases(errorRoot, Glob("bugsInputError/**.exec")) { args: Array[String] => | ||
assertThrows[InputPosError] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of the fix, this testcase should move to a different directory: inputError
. I think the bugsInputError
directory is for bugs in the error reporting code (which we are fixing now).
Apart from moving to the inputError
directory, no other changes should be required.
Hi @hrj , I hope these changes are as you had them in mind. Cheers, gerdreiss
Edit by hrj:
closes #130