[Bank account]: Add canonical data #2192
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,382 @@ | ||
| { | ||
| "exercise": "bank-account", | ||
| "comments": [ | ||
| "This exercise is a good candidate to practice concurrency, which", | ||
| "may not be available or possible in the implementing language track.", | ||
| "It is possible to implement this exercise without concurrency." | ||
| ], | ||
| "cases": [ | ||
| { | ||
| "uuid": "983a1528-4ceb-45e5-8257-8ce01aceb5ed", | ||
| "description": "Newly opened account has zero balance", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": 0 | ||
| }, | ||
| { | ||
| "uuid": "e88d4ec3-c6bf-4752-8e59-5046c44e3ba7", | ||
| "description": "Single deposit", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 100 | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": 100 | ||
| }, | ||
| { | ||
| "uuid": "3d9147d4-63f4-4844-8d2b-1fee2e9a2a0d", | ||
| "description": "Multiple deposits", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 100 | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 50 | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": 150 | ||
| }, | ||
| { | ||
| "uuid": "08f1af07-27ae-4b38-aa19-770bde558064", | ||
| "description": "Withdraw once", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 100 | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": 75 | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": 25 | ||
| }, | ||
| { | ||
| "uuid": "6f6d242f-8c31-4ac6-8995-a90d42cad59f", | ||
| "description": "Withdraw twice", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 100 | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": 80 | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": 20 | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": 0 | ||
| }, | ||
| { | ||
| "uuid": "45161c94-a094-4c77-9cec-998b70429bda", | ||
| "description": "Can do multiple operations sequentially", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 100 | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 110 | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": 200 | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 60 | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": 50 | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": 20 | ||
| }, | ||
| { | ||
| "uuid": "f9facfaa-d824-486e-8381-48832c4bbffd", | ||
| "description": "Cannot check balance of closed account", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "close" | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "account not open" } | ||
| }, | ||
| { | ||
| "uuid": "7a65ba52-e35c-4fd2-8159-bda2bde6e59c", | ||
| "description": "Cannot deposit into closed account", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "close" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 50 | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "account not open" } | ||
| }, | ||
| { | ||
| "uuid": "a0a1835d-faae-4ad4-a6f3-1fcc2121380b", | ||
| "description": "Cannot deposit into unopened account", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 50 | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "account not open" } | ||
| }, | ||
| { | ||
| "uuid": "570dfaa5-0532-4c1f-a7d3-0f65c3265608", | ||
| "description": "Cannot withdraw from closed account", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "close" | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": 50 | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "account not open" } | ||
| }, | ||
| { | ||
| "uuid": "c396d233-1c49-4272-98dc-7f502dbb9470", | ||
| "description": "Cannot close an account that was not opened", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "close" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "account not open" } | ||
| }, | ||
| { | ||
| "uuid": "c06f534f-bdc2-4a02-a388-1063400684de", | ||
| "description": "Cannot open an already opened account", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "open" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "account already open" } | ||
| }, | ||
| { | ||
| "uuid": "0722d404-6116-4f92-ba3b-da7f88f1669c", | ||
| "description": "Reopened account does not retain balance", | ||
|
There was a problem hiding this comment. This is a new test case AFAICT, correct? This test case and a0a1835d-faae-4ad4-a6f3-1fcc2121380b are the two test cases of which I'm not sure about. Most existing implementations of this exercise don't have these check I think. There was a problem hiding this comment. Python has both of those tests, I can't speak although for how other tracks have done it. There was a problem hiding this comment. Ah, that's one implementation that I didn't do. Let's keep them then. |
||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 50 | ||
| }, | ||
| { | ||
| "operation": "close" | ||
| }, | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": 0 | ||
| }, | ||
| { | ||
| "uuid": "ec42245f-9361-4341-8231-a22e8d19c52f", | ||
| "description": "Cannot withdraw more than deposited", | ||
|
|
||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 25 | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": 50 | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "amount must be less than balance" } | ||
| }, | ||
| { | ||
| "uuid": "4f381ef8-10ef-4507-8e1d-0631ecc8ee72", | ||
| "description": "Cannot withdraw negative", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 100 | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": -50 | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "amount must be greater than 0" } | ||
| }, | ||
| { | ||
| "uuid": "d45df9ea-1db0-47f3-b18c-d365db49d938", | ||
| "description": "Cannot deposit negative", | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "deposit", | ||
| "amount": -50 | ||
| } | ||
| ] | ||
| }, | ||
| "expected": { "error": "amount must be greater than 0" } | ||
| }, | ||
| { | ||
| "uuid": "ba0c1e0b-0f00-416f-8097-a7dfc97871ff", | ||
| "description": "Can handle concurrent transactions", | ||
| "comments": [ | ||
| "This test is a good candidate to practice concurrency, which", | ||
| "may not be available or possible in the implementing language track.", | ||
| "Concurrency testing is often made by running the same operations in", | ||
| "parallel, and then checking that the final state is as expected." | ||
| ], | ||
| "scenarios": ["concurrent"], | ||
| "property": "bankAccount", | ||
| "input": { | ||
| "operations": [ | ||
| { | ||
| "operation": "open" | ||
| }, | ||
| { | ||
| "operation": "concurrent", | ||
| "number": 1000, | ||
| "operations": [ | ||
| { | ||
| "operation": "deposit", | ||
| "amount": 1 | ||
| }, | ||
| { | ||
| "operation": "withdraw", | ||
| "amount": 1 | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "operation": "balance" | ||
| } | ||
| ] | ||
| }, | ||
| "expected": 0 | ||
| } | ||
| ] | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Fortunately, in all cases where this file has an
"expected": { "error": "..." }, it's always the case that it was the last operation that would cause the error.If there were ever a case in the future where an operation other than the last one should cause the error, then putting it here would be unclear because it is unspecific about which operation should cause the error. In that case, it would be necessary to improve clarity by putting this
"expected": { "error": "..." }in the operation that should cause the error, instead of here.I cannot predict at this time whether there will ever be such a case in the future, so I currently recommend that reviewers continue to consider this pull request in its current form ready to be Approved (in the GitHub sense of that word), rather than refusing to Approve it based on these grounds.
If we were to discover that there is likely a need for such cases in the future, I would recommend making the change now so that all these cases don't have to be reimplemented.
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.
It might be a good idea to make this convention clear though - the comments would say something like "if an error is expected, the last operation is the one that caused it; all operations other than the last one should succeed"
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 will see if other reviewers feels like there needs to be a structure change if not I will add that as a comment that it is the last one that should always be the action which will raise error