Skip to content

KF-34 add tests for FltCommunicationPort#80

Open
Kellesi wants to merge 1 commit intomainfrom
KF-34-test-FltCommunicationPort
Open

KF-34 add tests for FltCommunicationPort#80
Kellesi wants to merge 1 commit intomainfrom
KF-34-test-FltCommunicationPort

Conversation

@Kellesi
Copy link
Collaborator

@Kellesi Kellesi commented Aug 27, 2025

@Kellesi Kellesi requested a review from Copilot August 27, 2025 12:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive unit tests for the FltCommunicationPort class using mock objects to avoid dependencies on actual Windows kernel APIs. The implementation introduces a dependency injection pattern through IWinApi interface abstraction to enable testability.

  • Introduces IWinApi interface and WinApi wrapper class for Windows API abstraction
  • Creates WinApiMock for testing FltCommunicationPort functionality
  • Adds comprehensive test coverage for FltCommunicationPort creation, error handling, and cleanup scenarios

Reviewed Changes

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

Show a summary per file
File Description
test/pch.h Adds fltKernel.h include for filter manager types
test/WinApiMock.h Implements mock Windows API for unit testing
test/FltCommunicationPortTest.cpp Comprehensive test suite for FltCommunicationPort
test/CMakeLists.txt Adds new test files to build configuration
include/kf/WinApi.h Production wrapper implementing IWinApi interface
include/kf/IWinApi.h Interface definition for Windows API abstraction
include/kf/FltCommunicationPort.h Updates to use IWinApi dependency injection

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +106 to +107
static bool freeCalled = false;
apiMock.m_funcs.FltFreeSecurityDescriptor = [](PSECURITY_DESCRIPTOR) { freeCalled = true; };
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

Static variable 'freeCalled' could cause race conditions or state pollution between test runs. Consider using a non-static variable or resetting it before each test.

Suggested change
static bool freeCalled = false;
apiMock.m_funcs.FltFreeSecurityDescriptor = [](PSECURITY_DESCRIPTOR) { freeCalled = true; };
bool freeCalled = false;
apiMock.m_funcs.FltFreeSecurityDescriptor = [&freeCalled](PSECURITY_DESCRIPTOR) { freeCalled = true; };

Copilot uses AI. Check for mistakes.
Comment on lines +166 to +167
static bool freeCalled = false;
apiMock.m_funcs.FltCloseCommunicationPort = [](PFLT_PORT) { freeCalled = true; };
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

Another static variable 'freeCalled' that could cause race conditions or state pollution between test runs. Consider using a non-static variable or resetting it before each test.

Suggested change
static bool freeCalled = false;
apiMock.m_funcs.FltCloseCommunicationPort = [](PFLT_PORT) { freeCalled = true; };
bool freeCalled = false;
apiMock.m_funcs.FltCloseCommunicationPort = [&freeCalled](PFLT_PORT) { freeCalled = true; };

Copilot uses AI. Check for mistakes.
@Kellesi Kellesi force-pushed the KF-34-test-FltCommunicationPort branch from 24265eb to 0c2d2f3 Compare August 27, 2025 12:20
@Kellesi Kellesi requested a review from belyshevdenis August 27, 2025 12:26
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.

2 participants