Fixes #571 - Notify Users about out of date pack #590

BenWalker01 · 2025-10-21T17:47:51Z

447341769-3b0152d7-a479-4fcd-9809-110e99e71aea

Fixes #571

kristiankunc

I'd honestly love to see at least partial test coverage on this.

BenWalker01 · 2025-10-21T18:24:57Z

I'll write some in a bit

kristiankunc · 2025-10-21T18:26:25Z

The copilot comments about the files also look sensible, we can just throw an error instantly (which can be surpressed as I assume we don't want to enforce it 100%) + the rest are some nitpicks

…itialization

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-30T23:44:02Z

test/plugin/filestatus/FileStatusModuleTest.cpp

+#include "filestatus/FileStatusModule.h"
+#include "bootstrap/PersistenceContainer.h"
+#include "curl/CurlInterface.h"
+#include "curl/CurlRequest.h"
+#include "curl/CurlResponse.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <fstream>
+#include <filesystem>
+
+using ::testing::NiceMock;
+using ::testing::Return;
+using ::testing::Test;
+
+namespace UKControllerPluginTest {
+    namespace FileStatus {
+
+        // Mock Curl interface
+        class MockCurlInterface : public UKControllerPlugin::Curl::CurlInterface
+        {
+            public:
+            MOCK_METHOD(
+                UKControllerPlugin::Curl::CurlResponse,
+                MakeCurlRequest,
+                (const UKControllerPlugin::Curl::CurlRequest& request),
+                (override));
+        };
+
+        class FileStatusModuleTest : public Test
+        {
+            protected:
+            virtual void SetUp()
+            {
+                // Create temp directory for test files
+                testDir = std::filesystem::temp_directory_path() / "filestatus_test";
+                std::filesystem::create_directories(testDir);
+            }
+
+            virtual void TearDown()
+            {
+                // Clean up temp files
+                if (std::filesystem::exists(testDir)) {
+                    std::filesystem::remove_all(testDir);
+                }
+                if (std::filesystem::exists("./UK")) {
+                    std::filesystem::remove_all("./UK");
+                }
+            }
+
+            std::filesystem::path testDir;
+        };
+
+        // Test FetchPackVersion with successful response
+        TEST_F(FileStatusModuleTest, FetchPackVersionReturnsVersionOnSuccess)
+        {
+            NiceMock<MockCurlInterface> mockCurl;
+            UKControllerPlugin::Curl::CurlResponse response("abc123def456", false, 200);
+
+            EXPECT_CALL(mockCurl, MakeCurlRequest).WillOnce(Return(response));
+
+            std::string result = UKControllerPlugin::FileStatus::FetchPackVersion(mockCurl);
+            EXPECT_EQ(result, "abc123def456");
+        }
+
+        // Test FetchPackVersion with whitespace trimming
+        TEST_F(FileStatusModuleTest, FetchPackVersionTrimsWhitespace)
+        {
+            NiceMock<MockCurlInterface> mockCurl;
+            UKControllerPlugin::Curl::CurlResponse response("  abc123def456  \n\t", false, 200);
+
+            EXPECT_CALL(mockCurl, MakeCurlRequest).WillOnce(Return(response));
+
+            std::string result = UKControllerPlugin::FileStatus::FetchPackVersion(mockCurl);
+            EXPECT_EQ(result, "abc123def456");
+        }
+
+        // Test FetchPackVersion with non-200 status code
+        TEST_F(FileStatusModuleTest, FetchPackVersionReturnsEmptyOnBadStatusCode)
+        {
+            NiceMock<MockCurlInterface> mockCurl;
+            UKControllerPlugin::Curl::CurlResponse response("Not Found", false, 404);
+
+            EXPECT_CALL(mockCurl, MakeCurlRequest).WillOnce(Return(response));
+
+            std::string result = UKControllerPlugin::FileStatus::FetchPackVersion(mockCurl);
+            EXPECT_EQ(result, "");
+        }
+
+        // Test FetchPackVersion with exception
+        TEST_F(FileStatusModuleTest, FetchPackVersionReturnsEmptyOnException)
+        {
+            NiceMock<MockCurlInterface> mockCurl;
+
+            EXPECT_CALL(mockCurl, MakeCurlRequest).WillOnce(::testing::Throw(std::runtime_error("Network error")));
+
+            std::string result = UKControllerPlugin::FileStatus::FetchPackVersion(mockCurl);
+            EXPECT_EQ(result, "");
+        }
+
+    } // namespace FileStatus
+} // namespace UKControllerPluginTest


Missing test coverage for CheckSectorFileProviderFile function. This function contains complex logic for parsing the sector file provider configuration and URL extraction that should be tested, especially the edge case where the line is exactly "URL:" without any actual URL content.

src/plugin/filestatus/FileStatusModule.cpp

src/plugin/filestatus/FileStatusModule.h

src/plugin/filestatus/FileStatusModule.cpp

Copilot · 2025-11-30T23:44:04Z

test/plugin/filestatus/FileStatusModuleTest.cpp

+#include "filestatus/FileStatusModule.h"
+#include "bootstrap/PersistenceContainer.h"
+#include "curl/CurlInterface.h"
+#include "curl/CurlRequest.h"
+#include "curl/CurlResponse.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <fstream>
+#include <filesystem>
+
+using ::testing::NiceMock;
+using ::testing::Return;
+using ::testing::Test;
+
+namespace UKControllerPluginTest {
+    namespace FileStatus {
+
+        // Mock Curl interface
+        class MockCurlInterface : public UKControllerPlugin::Curl::CurlInterface
+        {
+            public:
+            MOCK_METHOD(
+                UKControllerPlugin::Curl::CurlResponse,
+                MakeCurlRequest,
+                (const UKControllerPlugin::Curl::CurlRequest& request),
+                (override));
+        };
+
+        class FileStatusModuleTest : public Test
+        {
+            protected:
+            virtual void SetUp()
+            {
+                // Create temp directory for test files
+                testDir = std::filesystem::temp_directory_path() / "filestatus_test";
+                std::filesystem::create_directories(testDir);
+            }
+
+            virtual void TearDown()
+            {
+                // Clean up temp files
+                if (std::filesystem::exists(testDir)) {
+                    std::filesystem::remove_all(testDir);
+                }
+                if (std::filesystem::exists("./UK")) {
+                    std::filesystem::remove_all("./UK");
+                }
+            }
+
+            std::filesystem::path testDir;
+        };
+
+        // Test FetchPackVersion with successful response
+        TEST_F(FileStatusModuleTest, FetchPackVersionReturnsVersionOnSuccess)
+        {
+            NiceMock<MockCurlInterface> mockCurl;
+            UKControllerPlugin::Curl::CurlResponse response("abc123def456", false, 200);
+
+            EXPECT_CALL(mockCurl, MakeCurlRequest).WillOnce(Return(response));
+
+            std::string result = UKControllerPlugin::FileStatus::FetchPackVersion(mockCurl);
+            EXPECT_EQ(result, "abc123def456");
+        }
+
+        // Test FetchPackVersion with whitespace trimming
+        TEST_F(FileStatusModuleTest, FetchPackVersionTrimsWhitespace)
+        {
+            NiceMock<MockCurlInterface> mockCurl;
+            UKControllerPlugin::Curl::CurlResponse response("  abc123def456  \n\t", false, 200);
+
+            EXPECT_CALL(mockCurl, MakeCurlRequest).WillOnce(Return(response));
+
+            std::string result = UKControllerPlugin::FileStatus::FetchPackVersion(mockCurl);
+            EXPECT_EQ(result, "abc123def456");
+        }
+
+        // Test FetchPackVersion with non-200 status code
+        TEST_F(FileStatusModuleTest, FetchPackVersionReturnsEmptyOnBadStatusCode)
+        {
+            NiceMock<MockCurlInterface> mockCurl;
+            UKControllerPlugin::Curl::CurlResponse response("Not Found", false, 404);
+
+            EXPECT_CALL(mockCurl, MakeCurlRequest).WillOnce(Return(response));
+
+            std::string result = UKControllerPlugin::FileStatus::FetchPackVersion(mockCurl);
+            EXPECT_EQ(result, "");
+        }
+
+        // Test FetchPackVersion with exception
+        TEST_F(FileStatusModuleTest, FetchPackVersionReturnsEmptyOnException)
+        {
+            NiceMock<MockCurlInterface> mockCurl;
+
+            EXPECT_CALL(mockCurl, MakeCurlRequest).WillOnce(::testing::Throw(std::runtime_error("Network error")));
+
+            std::string result = UKControllerPlugin::FileStatus::FetchPackVersion(mockCurl);
+            EXPECT_EQ(result, "");
+        }
+
+    } // namespace FileStatus
+} // namespace UKControllerPluginTest


Missing test coverage for CheckPackVersion function. The test file only covers FetchPackVersion, but CheckPackVersion contains critical logic including file reading, version comparison, and message box display that should be tested.

Copilot · 2025-11-30T23:44:04Z

src/plugin/filestatus/FileStatusModule.cpp

+            }
+
+        } catch (const std::exception& e) {
+            LogError("Exception when checking UK Controller Pack version");


The error message lacks detail about what exception occurred. It should include the exception message using e.what() to aid in debugging, similar to the error handling in FetchPackVersion on line 42.

Suggested change

LogError("Exception when checking UK Controller Pack version");

LogError("Exception when checking UK Controller Pack version: " + std::string(e.what()));

Copilot · 2025-11-30T23:44:04Z

src/plugin/filestatus/FileStatusModule.cpp

+                LogError("Could not find URL in sector file provider");
+            }
+
+            if (extractedUrl != SECTOR_FILE_DOWNLOADER_LINK) {


The comparison should trim whitespace from extractedUrl before comparing it to SECTOR_FILE_DOWNLOADER_LINK. The current implementation will fail if the URL in the file has leading/trailing whitespace, even though it may be functionally correct.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Add filestatus module

67789b3

kristiankunc requested a review from Copilot October 21, 2025 18:22

This comment was marked as outdated.

Sign in to view

kristiankunc reviewed Oct 21, 2025

View reviewed changes

BenWalker01 added 6 commits October 21, 2025 19:47

fix stuff from review

aff9520

update include path

ebdf86f

? more path updates

445fcc1

correct inc

7798868

Refactor FileStatusModuleTest to use ModuleBootstrap for container in…

bd528f5

…itialization

oops

d9c59f3

kristiankunc changed the title ~~Fixs #571 - Notify Users about out of date pack~~ Fixes #571 - Notify Users about out of date pack Oct 27, 2025

BenWalker01 added 6 commits November 4, 2025 20:42

update tests

912832f

correct mistakes

84d55e0

reset

67c2f75

help test

7695645

add test

b392460

update tests for FileStatusModule

3c6e56c

kristiankunc requested a review from Copilot November 30, 2025 23:41

Copilot started reviewing on behalf of kristiankunc November 30, 2025 23:41 View session

Copilot finished reviewing on behalf of kristiankunc November 30, 2025 23:43

Copilot AI reviewed Nov 30, 2025

View reviewed changes

Apply suggestion from @Copilot

8662813

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixes #571 - Notify Users about out of date pack #590

Fixes #571 - Notify Users about out of date pack #590

Uh oh!

BenWalker01 commented Oct 21, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

kristiankunc left a comment

Uh oh!

BenWalker01 commented Oct 21, 2025

Uh oh!

kristiankunc commented Oct 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 30, 2025

Uh oh!

Copilot AI Nov 30, 2025

Uh oh!

Copilot AI Nov 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	LogError("Exception when checking UK Controller Pack version");
	LogError("Exception when checking UK Controller Pack version: " + std::string(e.what()));

Fixes #571 - Notify Users about out of date pack #590

Are you sure you want to change the base?

Fixes #571 - Notify Users about out of date pack #590

Uh oh!

Conversation

BenWalker01 commented Oct 21, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

kristiankunc left a comment

Choose a reason for hiding this comment

Uh oh!

BenWalker01 commented Oct 21, 2025

Uh oh!

kristiankunc commented Oct 21, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants