Skip to content

fix: mock_core server build#1890

Open
JoshuaSBrown wants to merge 1 commit intodevelfrom
1854-mock-core-fix
Open

fix: mock_core server build#1890
JoshuaSBrown wants to merge 1 commit intodevelfrom
1854-mock-core-fix

Conversation

@JoshuaSBrown
Copy link
Collaborator

@JoshuaSBrown JoshuaSBrown commented Mar 12, 2026

Ticket

Description

How Has This Been Tested?

Artifacts (if appropriate):

Tasks

  • - A description of the PR has been provided, and a diagram included if it is a new feature.
  • - Formatter has been run
  • - CHANGELOG comment has been added
  • - Labels have been assigned to the pr
  • - A reviwer has been added
  • - A user has been assigned to work on the pr
  • - If new feature a unit test has been added

Summary by Sourcery

Fix mock_core test server version reporting and ensure its version header is generated during test builds.

Bug Fixes:

  • Correct version namespace used for mock_core component version in the test client worker so the server reports the intended version values.

Build:

  • Generate tests/mock_core/Version.hpp from Version.hpp.in via CMake during test configuration so the mock_core server can be built with the expected version header.

Tests:

  • Adjust mock_core client worker in tests to use the mock_core version constants rather than the core library versions.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Mar 12, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Fixes the mock_core test server build by aligning the version reporting in ClientWorker with the mock_core-local version namespace and wiring CMake to generate a mock_core Version.hpp header from a template during test configuration.

File-Level Changes

Change Details Files
Align mock_core ClientWorker version response with mock_core-local version constants instead of core library version constants.
  • Updated version reply to use version::MAJOR/MINOR/PATCH in procVersionRequest instead of core::version::MAJOR/MINOR/PATCH to remove the dependency on the core library version namespace.
tests/mock_core/ClientWorker.cpp
Generate a mock_core Version.hpp header for tests at configure time.
  • Added a CMake configure_file step to generate mock_core/Version.hpp from Version.hpp.in when integration tests are enabled, ensuring the mock_core test server has a version header available at build time.
  • Introduced a new Version.hpp.in template file in tests/mock_core as the source for the generated Version.hpp header.
tests/CMakeLists.txt
tests/mock_core/Version.hpp.in

Possibly linked issues

  • #[Authz, GridFTP]: PR fixes mock_core server build and versioning, directly supporting the mocked core service requested in the issue.

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. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the 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 exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

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
Contributor

@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 - I've left some high level feedback:

  • The configure_file call writes Version.hpp into ${CMAKE_CURRENT_SOURCE_DIR}; it would be more robust to generate this into the binary tree (e.g. ${CMAKE_CURRENT_BINARY_DIR}/mock_core/Version.hpp) to avoid modifying the source tree and accidentally committing generated files.
  • If the mock_core build and tests currently include Version.hpp from the source directory, consider updating include paths to consume the generated header from the binary directory instead, so configuration-specific values are correctly picked up per build.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `configure_file` call writes `Version.hpp` into `${CMAKE_CURRENT_SOURCE_DIR}`; it would be more robust to generate this into the binary tree (e.g. `${CMAKE_CURRENT_BINARY_DIR}/mock_core/Version.hpp`) to avoid modifying the source tree and accidentally committing generated files.
- If the mock_core build and tests currently include `Version.hpp` from the source directory, consider updating include paths to consume the generated header from the binary directory instead, so configuration-specific values are correctly picked up per build.

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.

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.

1 participant