search

microsoft / vscode-cmake-tools

CMake integration in Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=vector-of-bool.cmake-tools
MIT License
1.43k stars 430 forks source link

Tree view for tests using test suite delimiter #3759

Closed vinocher-bc closed 1 week ago

vinocher-bc commented 1 month ago

This change addresses item #3156

This changes visible behavior

The following changes are proposed:

vinocher-bc commented 1 month ago

@microsoft-github-policy-service agree [company="{your company}"]

@microsoft-github-policy-service agree company="The Browser Company"

vinocher-bc commented 1 month ago

@microsoft-github-policy-service agree [company="{your company}"]

@microsoft-github-policy-service agree company="The Browser Company"

tristanlabelle commented 3 weeks ago

@gcampbell-msft , can you help review this?

vinocher-bc commented 2 weeks ago

@gcampbell-msft could you please review? Thanks.

gcampbell-msft commented 2 weeks ago

@vinocher-bc @tristanlabelle Overall, I think this PR looks good! I've left comments on some things that need to be changed, but overall I don't have any reservations about the PR.

My only request would be to ensure that it's not regressing anything. @ebouteillon @nickassink @triou-harmonicinc @Jepsson, Could you all test this PR against your use cases? Or at least make a note and a point to test this once it merges into pre-release? You don't have to necessarily test the delimiter, I believe @vinocher-bc and @tristanlabelle have done that, but please ensure that it doesn't regress your use cases. Thanks

ebouteillon commented 1 week ago

Hi @gcampbell-msft , this feature looks interesting. Can you share a vsix with it?

gcampbell-msft commented 1 week ago

Hi @gcampbell-msft , this feature looks interesting. Can you share a vsix with it?

Sure! @vinocher-bc Could you let me know when you've updated the PR with the changes from my comments and when the build succeeds?

vinocher-bc commented 1 week ago

Sure! @vinocher-bc Could you let me know when you've updated the PR with the changes from my comments and when the build succeeds?

@gcampbell-msft, Thanks for looking. However, I can't see your comments in this PR.

gcampbell-msft commented 1 week ago

Sure! @vinocher-bc Could you let me know when you've updated the PR with the changes from my comments and when the build succeeds?

@gcampbell-msft, Thanks for looking. However, I can't see your comments in this PR.

You should be able to see them in the comments list, are you not able to see it? image

vinocher-bc commented 1 week ago

You should be able to see them in the comments list, are you not able to see it?

@gcampbell-msft, I can't see them. Appears that they are pending in the screen shot, so may need to be added to a review.

vinocher-bc commented 1 week ago

@gcampbell-msft, I've addressed your comments. Thanks.

triou-harmonicinc commented 1 week ago

My only request would be to ensure that it's not regressing anything. @ebouteillon @nickassink @triou-harmonicinc @Jepsson, Could you all test this PR against your use cases? Or at least make a note and a point to test this once it merges into pre-release? You don't have to necessarily test the delimiter, I believe @vinocher-bc and @tristanlabelle have done that, but please ensure that it doesn't regress your use cases. Thanks

I tested the delimiter as I am very interested in that feature as well. While the UI works very well, running a subset of the test in the hierarchy does not seem to work properly. It trigger a ctest command without any regex and runs all the test. Note that I use allowParallelJobs option.

gcampbell-msft commented 1 week ago

My only request would be to ensure that it's not regressing anything. @ebouteillon @nickassink @triou-harmonicinc @Jepsson, Could you all test this PR against your use cases? Or at least make a note and a point to test this once it merges into pre-release? You don't have to necessarily test the delimiter, I believe @vinocher-bc and @tristanlabelle have done that, but please ensure that it doesn't regress your use cases. Thanks

I tested the delimiter as I am very interested in that feature as well. While the UI works very well, running a subset of the test in the hierarchy does not seem to work properly. It trigger a ctest command without any regex and runs all the test. Note that I use allowParallelJobs option.

I know that there has been some confusion about how this is expected to work, could you describe what you are seeing? A video of what you've found would be helpful

triou-harmonicinc commented 1 week ago

@gcampbell-msft I reproduced with a gtest cmake example repo I forked : https://github.com/triou-harmonicinc/gtest-cmake-example tree_view_issue I was expecting to run only the 3 FooTest but all tests are running

gcampbell-msft commented 1 week ago

@gcampbell-msft I reproduced with a gtest cmake example repo I forked : https://github.com/triou-harmonicinc/gtest-cmake-example tree_view_issue tree_view_issue I was expecting to run only the 3 FooTest but all tests are running

@triou-harmonicinc @vinocher-bc I made a small commit to fix the issue you are seeing. There was a small error where we weren't passing through a parameter in our recursive call. I tested your repro and it fixed it, but could you confirm?

vinocher-bc commented 1 week ago

@triou-harmonicinc @vinocher-bc I made a small commit to fix the issue you are seeing. There was a small error where we weren't passing through a parameter in our recursive call. I tested your repro and it fixed it, but could you confirm?

@gcampbell-msft, thanks for the fix! This scenario of running a test group was working for me previously too (ctest -R was being used to run the individual tests in the group.) I've verified that they continue to work after your commit.

triou-harmonicinc commented 1 week ago

@gcampbell-msft I reproduced with a gtest cmake example repo I forked : https://github.com/triou-harmonicinc/gtest-cmake-example tree_view_issue

    [

        ![tree_view_issue](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM)

    ](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM)

      [

      ](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM)

   [ ![tree_view_issue](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM) ](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM)

    [

        ![tree_view_issue](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM)

    ](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM)

      [

      ](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM)

   [ ](https://private-user-images.githubusercontent.com/144131485/341463838-04f92373-c127-4338-9584-6b0d417b266c.gif?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4OTg3MjUsIm5iZiI6MTcxODg5ODQyNSwicGF0aCI6Ii8xNDQxMzE0ODUvMzQxNDYzODM4LTA0ZjkyMzczLWMxMjctNDMzOC05NTg0LTZiMGQ0MTdiMjY2Yy5naWY_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNjIwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDYyMFQxNTQ3MDVaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT05MDVkYWViZDkxMzc1MzI1NThjMTlhMWYxMjYxMzhmMDZmYTNkM2Q4MDBmMDQ0ZDYwMDIyMDBlZGZkMGExYzFmJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.R5SgPevtXTMwjFERStVbLINJzCq6yOmx3OXXeRTpwUM) I was expecting to run only the 3 FooTest but all tests are running

@triou-harmonicinc @vinocher-bc I made a small commit to fix the issue you are seeing. There was a small error where we weren't passing through a parameter in our recursive call. I tested your repro and it fixed it, but could you confirm?

Fix confirmed with my reproducing repo and other more complex source code. Thanks !

vinocher-bc commented 1 week ago

@gcampbell-msft could you please help with the CI failure on Windows?