Skip to content

feat(options) cover store and execution cases#40

Merged
lwtsn merged 1 commit into
mainfrom
39-problem-lacking-test-coverage
Apr 8, 2026
Merged

feat(options) cover store and execution cases#40
lwtsn merged 1 commit into
mainfrom
39-problem-lacking-test-coverage

Conversation

@lwtsn

@lwtsn lwtsn commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Resolves: #39

Copilot AI review requested due to automatic review settings April 8, 2026 05:43
@lwtsn lwtsn linked an issue Apr 8, 2026 that may be closed by this pull request
@lwtsn
lwtsn merged commit ec828d0 into main Apr 8, 2026
1 of 4 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds/updates unit tests to improve coverage for the options store and executor, and makes existing Greeks aggregation assertions more robust to floating-point precision.

Changes:

  • Add a new options store test suite (including concurrent-access test) and benchmarks.
  • Add a new options executor test suite covering happy paths and input validation/error paths.
  • Update existing Greeks aggregation tests to use tolerant numeric comparisons.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/markets/options/views/views_test.go Use tolerant float assertions for portfolio Greeks aggregation.
pkg/markets/options/store/store_test.go New store suite covering positions, prices, Greeks/IV, aggregation, concurrency; adds benchmarks.
pkg/markets/options/executor/executor_test.go New executor suite covering PlaceOrder/CancelOrder success and validation failures.
pkg/markets/options/analytics/service_test.go Use tolerant float assertions for portfolio Greeks aggregation.
pkg/markets/options/activity/pnl_test.go Use tolerant float assertions for portfolio Greeks aggregation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +39 to +44
contract1 = optionsTypes.OptionContract{
Pair: btcPair,
Strike: 50000,
Expiration: expiration,
OptionType: "CALL",
}

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

The store’s contract keying logic is currently unsafe for non-integer or large strike values: pkg/markets/options/store/store.go uses floatToString(f) = string(rune(f)), which truncates decimals and can collapse many strikes to the same rune (and values > unicode.MaxRune collapse to U+FFFD). These tests only use small integer strikes, so they won’t catch key collisions/overwrites. Add a regression test that stores two contracts differing only by strike (e.g., fractional strikes and/or strikes above 0x10FFFF) and expects both positions/greeks to be retrievable independently; then update the store to use a stable string encoding for float strikes (e.g., strconv/decimal formatting) to make the test pass.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Problem: lacking test coverage

2 participants