Skip to content

Revert "Adding more unit tests to reach 80%"#963

Closed
rahulc-gh wants to merge 1 commit intodevelopfrom
revert-177-import/develop/ROCm_aqlprofile/gobhardw_code-cov
Closed

Revert "Adding more unit tests to reach 80%"#963
rahulc-gh wants to merge 1 commit intodevelopfrom
revert-177-import/develop/ROCm_aqlprofile/gobhardw_code-cov

Conversation

@rahulc-gh
Copy link
Collaborator

Reverts #177

Copilot AI review requested due to automatic review settings September 11, 2025 23:27
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 reverts the previously added unit tests for the aqlprofile project, removing test coverage improvements that were targeting 80% coverage. The revert removes multiple test suites and simplifies the build configuration.

  • Removes additional unit test executables for PM4 builders (pmc, spm, trace config, sqtt)
  • Removes test suites for core components (pm4 factory, logger, aql profile v2)
  • Simplifies CMakeLists.txt files by removing test configuration blocks

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
projects/aqlprofile/src/pm4/tests/CMakeLists.txt Removes multiple test executables and reverts to single gfx9-command-builder-test
projects/aqlprofile/src/pm4/spm_builder.h Removes unnecessary include directive
projects/aqlprofile/src/pm4/cmd_config.h Removes unnecessary include directive
projects/aqlprofile/src/core/tests/CMakeLists.txt Removes test configurations for pm4-factory, logger, and aql-profile-v2 tests
projects/aqlprofile/src/CMakeLists.txt Removes test subdirectory includes and configuration

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

${CMAKE_CURRENT_SOURCE_DIR}/gfx9_cmd_builder_tests.cpp
)
target_sources(gfx9-command-builder-test PRIVATE ${AQLPROFILE_GFX9_COMMAND_BUILDER_SOURCES})
target_sources(gfx9-command-builder-test PRIVATE ${AQLPROFILE_COMMAND_BUILDER_SOURCES})
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

The variable name AQLPROFILE_COMMAND_BUILDER_SOURCES doesn't match the variable being defined above (AQLPROFILE_COMMAND_BUILDER_SOURCES is set on line 8-10 but should likely be AQLPROFILE_GFX9_COMMAND_BUILDER_SOURCES to match the test name).

Copilot uses AI. Check for mistakes.
gtest_add_tests(
TARGET gfx9-command-builder-test
SOURCES ${AQLPROFILE_GFX9_COMMAND_BUILDER_SOURCES}
SOURCES ${AQLPROFILE_COMMAND_BUILDER_SOURCES}
Copy link

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

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

The SOURCES parameter references AQLPROFILE_COMMAND_BUILDER_SOURCES but this variable contains cmd_builder_tests.cpp while the test is named gfx9-command-builder-test, suggesting it should reference gfx9_cmd_builder_tests.cpp instead.

Copilot uses AI. Check for mistakes.
@bgopesh
Copy link
Contributor

bgopesh commented Sep 12, 2025

closing this as the problem is fixed by #961

@bgopesh bgopesh closed this Sep 12, 2025
@rahulc-gh rahulc-gh deleted the revert-177-import/develop/ROCm_aqlprofile/gobhardw_code-cov branch September 12, 2025 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants