Skip to content

testing: remove basics AccountData.OnlineAccountData conversion function#6313

Merged
cce merged 1 commit intoalgorand:masterfrom
cce:remove-basics-OnlineAccountData-converter
May 6, 2025
Merged

testing: remove basics AccountData.OnlineAccountData conversion function#6313
cce merged 1 commit intoalgorand:masterfrom
cce:remove-basics-OnlineAccountData-converter

Conversation

@cce
Copy link
Copy Markdown
Contributor

@cce cce commented May 5, 2025

Summary

We have a conversion function in the basics.AccountData type to make a basics.OnlineAccountData type, but it is only used by tests. The actual conversion used by non-test code occurs in ledgercore.AccountData. However tests continue to use this function for setting up small tests with fake accounts and mocked ledgers. This moves it to the data/basics/testing package to hopefully reduce confusion, and make it so non-test code will continue to not use this method.

Test Plan

Existing tests were updated and should still pass.

@cce cce force-pushed the remove-basics-OnlineAccountData-converter branch from 00ca637 to 461f232 Compare May 5, 2025 17:53
@cce cce requested a review from Copilot May 5, 2025 17:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR moves the conversion function for AccountData to OnlineAccountData out of the production code and into the testing package, updating tests across the repository to use the new basics_testing.OnlineAccountData conversion function.

  • Replaces all calls to ad.OnlineAccountData() with basics_testing.OnlineAccountData(ad) in test files.
  • Removes the OnlineAccountData conversion function from the production data/basics/userBalance.go file and reintroduces it only for testing purposes in data/basics/testing.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ledger/ledger_test.go Updated OnlineAccountData conversion function call in tests.
ledger/eval_simple_test.go Updated OnlineAccountData conversion function call in tests.
ledger/eval/eval_test.go Updated OnlineAccountData conversion function call in tests.
ledger/acctonline_test.go Updated OnlineAccountData conversion function call in tests.
data/committee/common_test.go Updated OnlineAccountData conversion function call in tests.
data/basics/userBalance_test.go Removed extra helper functions; tests adjusted for new conversion.
data/basics/userBalance.go Removed the OnlineAccountData conversion function from production.
data/basics/testing/userBalance_test.go Added tests for the new testing conversion function.
data/basics/testing/userBalance.go Introduced the OnlineAccountData conversion function for tests only.
agreement/fuzzer/ledger_test.go Updated OnlineAccountData conversion function call in tests.
agreement/common_test.go Updated OnlineAccountData conversion function call in tests.
agreement/agreementtest/simulate_test.go Updated OnlineAccountData conversion function call in tests.

@codecov
Copy link
Copy Markdown

codecov bot commented May 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.57%. Comparing base (8341e41) to head (461f232).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6313      +/-   ##
==========================================
- Coverage   51.60%   51.57%   -0.04%     
==========================================
  Files         649      649              
  Lines       87048    87048              
==========================================
- Hits        44923    44894      -29     
- Misses      39263    39284      +21     
- Partials     2862     2870       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cce cce merged commit e4c416a into algorand:master May 6, 2025
21 checks passed
@cce cce deleted the remove-basics-OnlineAccountData-converter branch May 6, 2025 13:43
cce added a commit to cce/go-algorand that referenced this pull request May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants