Skip to content
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

Add tests for decimal128 and timestamp #78

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

AlekSi
Copy link
Member

@AlekSi AlekSi commented Jan 10, 2025

No description provided.

@Copilot Copilot bot review requested due to automatic review settings January 10, 2025 10:19

Choose a reason for hiding this comment

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

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

@mergify mergify bot assigned AlekSi Jan 10, 2025
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.14%. Comparing base (6f61731) to head (fc527f0).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
+ Coverage   63.04%   63.14%   +0.09%     
==========================================
  Files          41       41              
  Lines        2395     2401       +6     
==========================================
+ Hits         1510     1516       +6     
  Misses        701      701              
  Partials      184      184              
Files with missing lines Coverage Δ
wirebson/timestamp.go 76.92% <100.00%> (+19.78%) ⬆️
Flag Coverage Δ
test 63.14% <100.00%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@AlekSi AlekSi requested a review from Copilot January 10, 2025 10:25

Choose a reason for hiding this comment

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

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

Files not reviewed (2)
  • go.mod: Language not supported
  • wirebson/bson.go: Evaluated as low risk
Comments suppressed due to low confidence (1)

wirebson/logging.go:338

  • [nitpick] The variable name 'H' is ambiguous. It should be renamed to 'High' for better readability.
b.WriteString("Decimal128(H:")
@AlekSi AlekSi marked this pull request as draft January 10, 2025 10:58
@AlekSi AlekSi requested a review from Copilot January 10, 2025 10:58
Copy link

@Copilot 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.

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

Files not reviewed (1)
  • go.mod: Language not supported

wireclient/wireclient_test.go Outdated Show resolved Hide resolved
@AlekSi AlekSi changed the title Add more tests for decimal128 Add tests for decimal128 and timestamp Jan 10, 2025
@AlekSi AlekSi requested a review from Copilot January 10, 2025 11:22

Choose a reason for hiding this comment

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

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

Files not reviewed (1)
  • go.mod: Language not supported
@AlekSi AlekSi requested a review from Copilot January 12, 2025 09:18
Copy link

@Copilot 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.

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

Files not reviewed (1)
  • go.mod: Language not supported
Comments suppressed due to low confidence (1)

wireclient/wireclient_test.go:216

  • The placeholder for the Timestamp test should be completed or removed.
t.Run("Timestamp", func(t *testing.T) { // FIXME })

wireclient/wireclient_test.go Show resolved Hide resolved
@AlekSi AlekSi requested a review from Copilot January 31, 2025 16:16
@AlekSi AlekSi marked this pull request as ready for review January 31, 2025 16:16
@AlekSi AlekSi enabled auto-merge (squash) January 31, 2025 16:16
@AlekSi AlekSi added the code/feature Some user-visible feature is not implemented yet label Jan 31, 2025

Choose a reason for hiding this comment

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

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

Files not reviewed (2)
  • go.mod: Language not supported
  • tools/go.mod: Language not supported
Comments suppressed due to low confidence (2)

wireclient/wireclient_test.go:211

  • Consider adding test cases for edge values of 'time' and 'increment' in the 'Timestamp' tests to ensure robustness.
t.Run("Timestamp", func(t *testing.T) {

wirebson/bson_test.go:1303

  • The comment on the t.Skip line should provide more context about why the test is being skipped.
t.Skip("https://github.com/FerretDB/wire/issues/49")
@AlekSi AlekSi requested review from a team, chilagrow and noisersup January 31, 2025 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/feature Some user-visible feature is not implemented yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

1 participant