Sync bank-account tests with problem specifications - #541
Conversation
|
This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested. If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos. For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping |
|
Hello. Thanks for opening a PR on Exercism 🙂 We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in. You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch. If you're interested in learning more about this auto-responder, please read this blog post. Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it. |
|
Hello 👋 Thanks for your PR. This repo does not currently have dedicated maintainers. Our cross-track maintainers team will attempt to review and merge your PR, but it will likely take longer for your PR to be reviewed. If you enjoy contributing to Exercism and have a track-record of doing so successfully, you might like to become an Exercism maintainer for this track. Please feel free to ask any questions, or chat to us about anything to do with this PR or the reviewing process on the Exercism forum. (cc @exercism/cross-track-maintainers) |
|
This is an unmaintained repository. Cross-track maintainers - feel free to merge. |
Description
This PR synchronizes the
bank-accountpractice exercise with thelatest canonical test data from the Exercism
problem-specificationsrepository.
During the synchronization process, the existing VB.NET exercise was
compared directly against the canonical
bank-accountspecificationrather than relying solely on the existing
.meta/tests.tomlrepresentation.
This comparison revealed that the existing exercise was based on an
older API and behavioral model centered around:
The current canonical specification instead models deposits and
withdrawals as distinct operations and defines explicit behavior for
account lifecycle, invalid operations, balance handling, insufficient
funds, and concurrent transactions.
Because the existing API could not faithfully represent the current
canonical behavior, this PR updates the exercise API, example solution,
test suite, and canonical test metadata together.
Motivation
The purpose of this change is to bring the VB.NET
bank-accountexercise into alignment with the current canonical specification.
The existing implementation treated positive and negative balance
changes through a single method:
For example:
could be used to deposit funds, while:
could be used to withdraw funds.
This model no longer matches the canonical specification.
The canonical exercise explicitly distinguishes:
and defines separate validation rules for each operation.
For example:
must be rejected as an invalid deposit rather than being interpreted as
a withdrawal.
Likewise:
must be rejected rather than increasing the account balance.
The canonical specification also defines account lifecycle behavior that
the old API did not model explicitly.
Therefore, simply adding the newly generated canonical tests would not
have been sufficient. The exercise itself needed to be updated so that
the canonical behavior could be represented correctly.
Canonical Test Synchronization
The current canonical
bank-accountspecification contains 17 testcases.
The canonical data was inspected directly from:
The existing track metadata was then synchronized using
configlet.The resulting:
contains the canonical test UUIDs and descriptions.
No canonical cases were intentionally excluded.
The metadata was verified with:
which reports:
This confirms that the track metadata is synchronized with the canonical
test data.
API Changes
Previous API
The previous exercise exposed:
The important limitation was that deposits and withdrawals were
represented using the same operation.
For example:
represented a deposit, while:
represented a withdrawal.
This made it impossible to faithfully represent canonical rules such as:
The single signed
UpdateBalanceparameter blurred the distinctionbetween these operations.
New API
The exercise now exposes:
This matches the conceptual model used by the canonical specification.
The API is now consistent across:
BankAccount.vb.meta/Example.vbBankAccountTests.vbThe old:
UpdateBalance(...)API has been removed from the exercise implementation and test suite.
BankAccount.vbThe student-facing exercise stub was updated to expose the new API.
The stub now requires students to implement:
The methods remain intentionally unimplemented for students.
The purpose of this file is to provide the interface students are
expected to implement.
The old:
UpdateBalance()method is no longer exposed.
This is important because the student-facing API must correspond to the
operations exercised by the canonical tests.
.meta/Example.vbThe example solution was substantially updated because the old
implementation could not satisfy the canonical behavior.
The new implementation maintains account state and protects it for
concurrent operations.
The implementation now handles:
The implementation also uses synchronization around shared account state
so that concurrent operations cannot corrupt the balance.
Account Lifecycle
Opening an account
Calling:
account.Open()on a closed account opens the account and initializes its balance to:
The implementation therefore guarantees that every newly opened account
starts with a clean balance.
Opening an already-open account
Calling:
account.Open()when the account is already open throws:
This corresponds to the canonical "already open" behavior.
Closing an account
Calling:
account.Close()on an open account closes the account and resets the internal balance.
The balance is therefore not retained between separate account sessions.
Closing an unopened account
Calling:
account.Close()before the account has been opened throws:
This is explicitly covered by the test suite.
Reopening an Account
The canonical specification requires that reopening an account starts a
new account session with a zero balance.
For example:
must result in:
The test suite explicitly verifies this behavior.
This prevents a previous account balance from surviving a close/open
cycle.
Balance Behavior
The
Balanceproperty represents the balance of an open account.Checking the balance while the account is closed is invalid.
For example:
throws:
The test intentionally opens and then closes the account so that it
tests the closed state rather than an account that was never opened.
Deposit Behavior
The new:
Deposit(amount)operation adds funds to the account.
For example:
results in:
Multiple deposits accumulate:
results in:
Deposits are rejected when the account is not open.
Negative Deposits
The canonical specification explicitly rejects negative deposits.
For example:
must throw:
This is one of the reasons the old
UpdateBalanceAPI could not simplybe retained.
Under the old API, a negative value could be interpreted as a
withdrawal.
Under the new API:
is unambiguously an invalid deposit.
Withdrawal Behavior
The new:
Withdraw(amount)operation subtracts funds from the current balance.
For example:
results in:
Multiple withdrawals are supported while sufficient funds remain.
Negative Withdrawals
Negative withdrawals are invalid.
For example:
throws:
This prevents a negative withdrawal from accidentally behaving like a
deposit.
Withdrawals Greater Than the Balance
The implementation prevents the account from becoming overdrawn.
For example:
throws:
The old
UpdateBalancemodel did not naturally enforce this distinctionbecause a negative balance change was simply another signed adjustment.
The new API allows the canonical rule to be expressed directly.
Operations on Closed Accounts
The canonical specification requires operations on closed accounts to
fail.
The test suite explicitly distinguishes a closed account from an account
that has never been opened.
Deposit after closing
results in:
Withdrawal after closing
also results in:
Checking balance after closing
results in:
These tests intentionally call
Open()followed byClose()so thatthey actually test the closed state.
Unopened Account Behavior
The canonical specification also contains cases where operations are
attempted on an account that has never been opened.
For example:
must fail with:
This is intentionally kept as a separate test from the closed-account
case.
The distinction is important because:
and:
are different state transitions, even though both reject operations.
Sequential Operations
The test suite covers combinations of operations.
For example:
must result in:
This verifies that the implementation maintains state correctly across
multiple sequential operations rather than only handling isolated
deposits or withdrawals.
Concurrency
The existing exercise already had a concurrency test, but it was based
on the old
UpdateBalanceAPI.The old pattern effectively performed:
repeated concurrently.
That test was updated to use the canonical API:
The updated test launches 1,000 concurrent tasks.
Each task performs:
The expected final balance is:
This directly exercises the canonical concurrency behavior.
Thread Safety
The example implementation protects shared account state using
SyncLock.Operations that read or modify the account state are synchronized.
This prevents race conditions between concurrent deposits and
withdrawals.
The synchronization is particularly important because the canonical test
intentionally performs operations from multiple concurrent tasks.
Without synchronization, simultaneous balance updates could result in
lost updates or an incorrect final balance.
Exact Error Messages
The example implementation uses the canonical error messages rather than
introducing track-specific alternatives.
The following messages are used:
The test suite verifies the exception messages explicitly.
For example:
This ensures that the implementation is not merely throwing an
exception, but is matching the expected canonical behavior.
Imports in
BankAccountTests.vbThe updated test suite uses types from namespaces that were not required
by the original test file.
The test file therefore explicitly imports the namespaces it uses:
System.Threading.Tasksis required for the concurrency coverage,including:
The
TaskAPI is used to execute the canonical concurrentdeposit/withdraw operations.
Xunitprovides the test framework APIs used throughout the file,including:
The original test file did not require these explicit imports in the
same way because the previous track/project setup already allowed the
original tests to resolve the required test framework types.
The imports are therefore part of the updated test file's requirements
rather than unrelated formatting changes.
BankAccountTests.vbThe existing test suite was rewritten around the new API.
The previous tests were based on the old
UpdateBalancebehavior andincluded cases where negative balance changes were used to represent
withdrawals.
Those tests were replaced with explicit deposit and withdrawal
operations.
The final suite contains 17 active tests covering the canonical
behavior.
The tests verify:
balance.
All tests are active.
No tests are skipped.
Canonical Case Mapping
The final test suite intentionally distinguishes cases that can
otherwise appear superficially similar.
For example:
Closed account
tests the behavior of an account that was opened and subsequently
closed.
Unopened account
tests the behavior of an account that has never been opened.
Both operations produce:
but they represent different canonical state transitions and therefore
remain separate tests.
This distinction was important during the synchronization because simply
counting test methods would not guarantee that all canonical behaviors
were represented correctly.
Metadata
The
.meta/tests.tomlfile was synchronized usingconfiglet.The file contains the canonical UUIDs and descriptions for all 17 cases.
The generated metadata was checked rather than manually treating the
previous metadata as authoritative.
No canonical test cases were excluded.
The resulting metadata was verified with:
and
configletreports:Verification
The complete exercise test suite was run with:
Result:
The canonical test metadata was also verified with:
Result:
The staged changes were checked with:
No whitespace errors were reported.
Files Changed
exercises/practice/bank-account/.meta/tests.tomlexercises/practice/bank-account/BankAccount.vbUpdateBalance-based student API.Openoperation.Closeoperation.Depositoperation.Withdrawoperation.Balanceproperty.exercises/practice/bank-account/.meta/Example.vbexercises/practice/bank-account/BankAccountTests.vbUpdateBalance-based tests.deposits and withdrawals.
Verification Summary
Check Result
Canonical test cases 17
Active VB.NET tests 17
Tests passed 17
Tests failed 0
Tests skipped 0
Build Successful
configlet syncUp to dategit diff --cached --checkCleanCanonical cases excluded 0
Conclusion
This PR brings the VB.NET
bank-accountpractice exercise intoalignment with the current canonical specification.
The primary behavioral change is replacing the legacy
UpdateBalancemodel with explicit
DepositandWithdrawoperations. This allows theexercise to correctly represent the canonical validation rules, account
lifecycle semantics, insufficient-balance behavior, and concurrent
transaction behavior.
The student-facing API, example solution, tests, and canonical metadata
have all been updated consistently.
The final exercise contains all 17 canonical test cases, with all 17
tests passing successfully.
configletconfirms that the test metadata is synchronized with thecurrent
problem-specificationsdata.Final verification
This synchronization is therefore complete and ready for review.
Closes #423