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 initial test case for globus api #1050

Open
wants to merge 1 commit into
base: 1066-bug-fix
Choose a base branch
from

Conversation

nedvedba
Copy link
Collaborator

@nedvedba nedvedba commented Nov 7, 2024

Summary by Sourcery

Tests:

  • Add initial test cases for the Globus API, including basic test cases for POST and GET requests.

Copy link

sourcery-ai bot commented Nov 7, 2024

Reviewer's Guide by Sourcery

This PR introduces the initial test structure for the Globus API by creating a new test file and updating the CMake configuration. The test file currently contains placeholder test cases for GET and POST operations.

Class diagram for the new Globus API test cases

classDiagram
    class GlobusAPITest {
        +testing_GlobusAPIPost()
        +testing_GlobusAPIGet()
    }
    note for GlobusAPITest "This class contains placeholder test cases for the Globus API GET and POST operations."
Loading

File-Level Changes

Change Details Files
Added new test suite for Globus API functionality
  • Created placeholder test cases for GET and POST operations
  • Set up basic test infrastructure with Boost Test framework
  • Included necessary header files for testing
core/server/tests/unit/test_GlobusAPI.cpp
Updated CMake configuration to include new test file
  • Added test_GlobusAPI to the list of test programs
core/server/tests/unit/CMakeLists.txt

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

@nedvedba nedvedba self-assigned this Nov 7, 2024
@nedvedba nedvedba added the Type: Bug Something isn't working label Nov 7, 2024
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:

  • The test cases are currently just placeholder assertions (BOOST_TEST(true)). Please implement actual test cases that verify the GlobusAPI functionality, including successful API calls, error conditions, and edge cases.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟡 Testing: 3 issues found
  • 🟢 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.

Comment on lines +20 to +21
BOOST_AUTO_TEST_CASE(testing_GlobusAPIPost) {
BOOST_TEST(true);
Copy link

Choose a reason for hiding this comment

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

issue (testing): Empty test case for GlobusAPIPost

This test case is currently just asserting true, which doesn't validate any actual functionality. Consider adding meaningful assertions that verify the POST request behavior, including successful requests, error handling, and edge cases like invalid input or network failures.

BOOST_AUTO_TEST_SUITE(GlobusAPITest)

BOOST_AUTO_TEST_CASE(testing_GlobusAPIPost) {
BOOST_TEST(true);
Copy link

Choose a reason for hiding this comment

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

issue (testing): Empty test case for GlobusAPIGet

This test case is currently just asserting true, which doesn't validate any actual functionality. Consider adding meaningful assertions that verify the GET request behavior, including successful responses, error handling, and edge cases like invalid endpoints or authentication failures.

Comment on lines +16 to +18
using namespace SDMS::Core;

BOOST_AUTO_TEST_SUITE(GlobusAPITest)
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Missing test setup and teardown

Consider adding BOOST_FIXTURE_TEST_SUITE or appropriate setup/teardown methods to handle any necessary test environment initialization (like mock HTTP clients, test credentials, etc.) and cleanup.

using namespace SDMS::Core;

struct GlobusAPITestFixture {
    GlobusAPITestFixture() {
        // Setup code would go here
    }
    ~GlobusAPITestFixture() {
        // Cleanup code would go here
    }
};

BOOST_FIXTURE_TEST_SUITE(GlobusAPITest, GlobusAPITestFixture)

@JoshuaSBrown JoshuaSBrown changed the base branch from release_June_2024 to devel November 7, 2024 19:24
@nedvedba nedvedba changed the base branch from devel to 1066-bug-fix November 11, 2024 10:26
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