Skip to content

Conversation

@kianmeng
Copy link
Contributor

@kianmeng kianmeng commented Dec 14, 2024

Adding missing validation and tests when making a new connection.

Summary by Sourcery

Refactor the Connection.new/1 function to include validation for required fields and add corresponding tests to ensure proper functionality and error handling.

Enhancements:

  • Add validation for required fields in the Connection.new/1 function to ensure all necessary fields are provided.

Tests:

  • Introduce new tests for the Connection.new/1 function to verify correct struct creation and error handling for missing or invalid data.

Adding missing validation and tests when making a new connection.
@sourcery-ai
Copy link

sourcery-ai bot commented Dec 14, 2024

Reviewer's Guide by Sourcery

The PR refactors the OpenApiTypesense.Connection.new/1 function to add proper validation for required connection fields and improves error handling. The implementation now validates that all required fields are present in the connection map and raises appropriate error messages when validation fails. The changes also include comprehensive test coverage for the connection creation functionality.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Added validation for required connection fields
  • Implemented field validation using required_fields/0 helper function
  • Added error handling for missing required fields
  • Added type validation to ensure connection parameter is a map
  • Updated function documentation with error case example
lib/open_api_typesense/connection.ex
Added comprehensive test coverage for connection creation
  • Added tests for default connection creation
  • Added tests for custom connection creation
  • Added tests for validation error cases
  • Added tests for invalid input type handling
test/open_api_typesense_connection_test.exs
Improved code documentation and formatting
  • Fixed function reference format in documentation
  • Improved code formatting and spacing
  • Added error case example in documentation
lib/open_api_typesense/connection.ex

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @kianmeng - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@coveralls
Copy link

Pull Request Test Coverage Report for Build a1b10d5ef3193d4b73abe7461886f991f3d5df69-PR-2

Details

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 98.921%

Totals Coverage Status
Change from base Build 26ceb5fe3fe7f38ca2d4a67d9dd490e3c9c2f50d: 0.02%
Covered Lines: 275
Relevant Lines: 278

💛 - Coveralls

@jaeyson
Copy link
Owner

jaeyson commented Dec 14, 2024

Hi @kianmeng, thanks for the PR. There's an error in CI test:

https://github.com/jaeyson/open_api_typesense/actions/runs/12332042761/job/34423378550?pr=2

  1) test new/1 with empty map raises ArgumentError (OpenApiTypesense.ConnectionTest)
Error:      test/open_api_typesense_connection_test.exs:34
     Wrong message for ArgumentError
     expected:
       "Missing required fields: [:port, :scheme, :host, :api_key]"
     actual:
       "Missing required fields: [:api_key, :host, :port, :scheme]"
     code: assert_raise ArgumentError, msg, fn -> Connection.new(%{}) end
     stacktrace:
       test/open_api_typesense_connection_test.exs:36: (test)

..................................................................
Finished in 0.6 seconds (0.6s async, 0.01s sync)
70 tests, 1 failure

Is it the order of the list [:api_key, :host, :port, :scheme]?

@jaeyson jaeyson merged commit 57d04d6 into jaeyson:main Dec 15, 2024
6 of 7 checks passed
@kianmeng
Copy link
Contributor Author

🥳 🥳 🥳 🥳 🥳

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.

3 participants