Skip to content

test: Add UTs for rsx::simple_array<T>#17136

Merged
kd-11 merged 2 commits into
RPCS3:masterfrom
kd-11:unit-tests
May 1, 2025
Merged

test: Add UTs for rsx::simple_array<T>#17136
kd-11 merged 2 commits into
RPCS3:masterfrom
kd-11:unit-tests

Conversation

@kd-11

@kd-11 kd-11 commented May 1, 2025

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread rpcs3/tests/test_simple_array.cpp Outdated
@Megamouse

Megamouse commented May 1, 2025

Copy link
Copy Markdown
Contributor

You have to add the file to cmakelists and vs

@kd-11

kd-11 commented May 1, 2025

Copy link
Copy Markdown
Contributor Author

You have to add the file to cmakelists and vs

I can't even see the other test files in vs. Where are they?

@Megamouse

Copy link
Copy Markdown
Contributor

I think I accidentally put them behind the GTestInstalled condition

<ItemGroup Condition="'$(GTestInstalled)' == 'true'">

@Megamouse

Copy link
Copy Markdown
Contributor

Change it to:

  <ItemGroup>
    <ClCompile Include="test.cpp">
      <ExcludedFromBuild Condition="'$(GTestInstalled)' != 'true'">true</ExcludedFromBuild>
    </ClCompile>
    <ClCompile Include="test_fmt.cpp">
      <ExcludedFromBuild Condition="'$(GTestInstalled)' != 'true'">true</ExcludedFromBuild>
    </ClCompile>
    <ClCompile Include="test_simple_array.cpp">
      <ExcludedFromBuild Condition="'$(GTestInstalled)' != 'true'">true</ExcludedFromBuild>
    </ClCompile>
  </ItemGroup>

@kd-11

kd-11 commented May 1, 2025

Copy link
Copy Markdown
Contributor Author

Change it to:

  <ItemGroup>
    <ClCompile Include="test.cpp">
      <ExcludedFromBuild Condition="'$(GTestInstalled)' != 'true'">true</ExcludedFromBuild>
    </ClCompile>
    <ClCompile Include="test_fmt.cpp">
      <ExcludedFromBuild Condition="'$(GTestInstalled)' != 'true'">true</ExcludedFromBuild>
    </ClCompile>
    <ClCompile Include="test_simple_array.cpp">
      <ExcludedFromBuild Condition="'$(GTestInstalled)' != 'true'">true</ExcludedFromBuild>
    </ClCompile>
  </ItemGroup>

2 things:

  1. The condition seems inverted.
  2. The GTestInstalled variable is false for me despite me having gtest adapter installed and the headers. Everything builds and runs fine if I ignore the condition.
  3. Scalability. I don't think it's a good idea to add these manual filters, we should use some other custom check to keep it from running in CI

@kd-11

kd-11 commented May 1, 2025

Copy link
Copy Markdown
Contributor Author

I added a custom var to skip unit tests in CI.

Comment thread rpcs3/tests/test_simple_array.cpp Outdated
@Megamouse Megamouse added RSX Unit Tests Anything related to unit tests labels May 1, 2025
Comment thread rpcs3/tests/rpcs3_test.vcxproj Outdated
Comment thread rpcs3/Emu/RSX/Common/simple_array.hpp Outdated
@kd-11 kd-11 marked this pull request as ready for review May 1, 2025 22:16
@kd-11 kd-11 merged commit b25276d into RPCS3:master May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RSX Unit Tests Anything related to unit tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants