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

created a test class header for database api #1048

Open
wants to merge 4 commits into
base: 1045-bug-fix
Choose a base branch
from

Conversation

nedvedba
Copy link
Collaborator

@nedvedba nedvedba commented Nov 7, 2024

Summary by Sourcery

Introduce a TestDatabaseAPI class to facilitate testing of database interactions and modify access specifiers in DatabaseAPI to support subclassing.

New Features:

  • Introduce a new TestDatabaseAPI class for testing purposes, providing methods for database interactions such as dbGet and dbPost.

Enhancements:

  • Change the access specifier of dbGet and dbPost methods in DatabaseAPI from private to protected to allow subclass access.

@nedvedba nedvedba added the Type: Bug Something isn't working label Nov 7, 2024
@nedvedba nedvedba self-assigned this Nov 7, 2024
Copy link

sourcery-ai bot commented Nov 7, 2024

Reviewer's Guide by Sourcery

This PR introduces a new test class header for the DatabaseAPI and modifies the access level of some DatabaseAPI methods to support testing. The implementation creates a new TestDatabaseAPI class that will be used for testing database operations, and changes the access modifiers in the original DatabaseAPI class to allow the test class to access necessary methods.

Class diagram for DatabaseAPI and TestDatabaseAPI

classDiagram
    class DatabaseAPI {
        +metricsPurge(uint32_t a_timestamp, LogContext)
        #dbGet(const char *a_url_path, const std::vector<std::pair<std::string, std::string>> &a_params, libjson::Value &a_result, LogContext, bool a_log = true)
        #dbPost(const char *a_url_path, const std::vector<std::pair<std::string, std::string>> &a_params, const std::string *a_body, libjson::Value &a_result, LogContext)
        -setAuthStatus(Anon::AuthStatusReply &a_reply, const libjson::Value &a_result)
        -setUserData(Auth::UserDataReply &a_reply, const libjson::Value &a_result)
    }

    class TestDatabaseAPI {
        +TestDatabaseAPI(const std::string &a_db_url, const std::string &a_db_user, const std::string &a_db_pass)
        +~TestDatabaseAPI()
        +dbGet(const char *a_url_path, const std::vector<std::pair<std::string, std::string>> &a_params, libjson::Value &a_result, LogContext, bool a_log = true)
        +dbGetRaw(const char *a_url_path, const std::vector<std::pair<std::string, std::string>> &a_params, std::string &a_result)
        +dbPost(const char *a_url_path, const std::vector<std::pair<std::string, std::string>> &a_params, const std::string *a_body, libjson::Value &a_result, LogContext)
    }

    TestDatabaseAPI --> DatabaseAPI
Loading

File-Level Changes

Change Details Files
Modified access modifiers in DatabaseAPI class to support testing
  • Changed access level of database operation methods from private to protected
  • Moved authentication-related methods to private section
core/server/DatabaseAPI.hpp
Created new TestDatabaseAPI class header
  • Defined class structure with constructor and destructor
  • Added database operation methods matching the protected methods from DatabaseAPI
  • Included necessary header files and namespace declarations
core/server/TestDatabaseAPI.hpp

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 @nedvedba - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider using inheritance instead of duplicating the database access methods. TestDatabaseAPI should extend DatabaseAPI if you need to override behavior for testing.
  • Rather than modifying access levels and duplicating code, consider using dependency injection and a mocking framework for testing. This would maintain better encapsulation and reduce code duplication.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 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.

namespace SDMS {
namespace Core {

class TestDatabaseAPI {
Copy link

Choose a reason for hiding this comment

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

suggestion: Consider having TestDatabaseAPI inherit from DatabaseAPI to avoid code duplication

The database access methods are currently duplicated between DatabaseAPI and TestDatabaseAPI. Since these methods are now protected in DatabaseAPI, you could inherit from it and override only the methods that need different behavior for testing.

class TestDatabaseAPI : public DatabaseAPI {

@nedvedba nedvedba changed the base branch from release_June_2024 to 1066-bug-fix November 11, 2024 10:24
@nedvedba nedvedba changed the base branch from 1066-bug-fix to 1045-bug-fix November 11, 2024 10:30
@JoshuaSBrown
Copy link
Collaborator

@par-hermes format

@JoshuaSBrown
Copy link
Collaborator

Just get unit test working .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants