Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Find Unused Step Definitions' command doesn't handle 'StepDefinition' attributes well #22

Closed
UL-ChrisGlew opened this issue Jun 11, 2024 · 12 comments
Labels
enhancement New feature or request

Comments

@UL-ChrisGlew
Copy link
Contributor

Used Visual Studio

Visual Studio 2022

Are the latest Visual Studio updates installed?

Yes

Content of reqnroll.json (if present)

{
"bindingCulture": {
"name": "en-GB"
},
"language": {
"feature": "en-GB"
}
}

Issue Description

Step definitions declared with the 'StepDefinition' attribute are showing multiple times inside the new 'Find Unused Step Definitions' menu option. These appear to show Given/Then/When usages, and implies that the Step Definition is unused - which is not always the case.

Is it possible to change this behavior to show which methods decorated with 'StepDefinition' are unused?

Steps to Reproduce

  1. Have a Reqnroll project
  2. Create a new 'Step Definition' file, adding the [Binding] attribute
  3. Create a new step definition, using the 'StepDefinition' attribute. For example: [StepDefinition("I can do the thing")]
    image
  4. Use this StepDefinition in a feature file. For example: Given I can do the thing
    image
  5. View the 'Find Unused Step Definitions' context on the Step Definition file.
  6. You will see:
    • [Then("I can do the thing")]
    • [When("I can do the thing")]
      image

Link to a project repository that reproduces the issue

No response

@gasparnagy
Copy link
Contributor

That makes sense.

Reqnroll internally generates three step definition binding from the [StepDefinition] attribute and this is what the VS extension receives. At that point it is not possible to tell if they were coming from the same attribute or separate ones, but if they belong to the exact same method and have the exact same expression we could assume that these were created using [StepDefinition].

The improvement could be done by modifying the UnusedStepDefinitionsFinder.FindUnused method (plus add tests for it to FindUnusedStepDefinitionsCommandTests).

I think the implementation should:

  • add an additional loop before the return (here)
  • go through the ones that are considered unused (stepDefUsageCounts.Where(x => x.Value == 0))
  • for each of these, check if there is a used one (stepDefUsageCounts.Where(x => x.Value > 0)) that has the same Expression and the same Implementation (ref equals).
  • If there is one, it should not be counted as unused

Do you want to try making a PR for this?

@gasparnagy gasparnagy added the enhancement New feature or request label Jun 11, 2024
@UL-ChrisGlew
Copy link
Contributor Author

@gasparnagy Sure I'm happy to contribute, just a couple of questions:

  1. Is it possible to debug the Visual Studio extension? I can't seem to debug/run from the Reqnroll.VisualStudio.Package project without installing a 'preview' package into Visual Studio?
  2. I don't seem to have rights to create a new branch in this repository, is that something that could be sorted?

Thanks!

@gasparnagy
Copy link
Contributor

@UL-ChrisGlew Thank you for taking care of this.

For the development experience & debug experience I have updated now the CONTRIBUTION.md guide. That should contain all information. If anything is unclear, please let me know.

For pushing the branch: normally new contributors fork the project and make a new branch in their own fork and create a Pull Request from there. But I have added you now to the contributors group (you need to accept the invite), so you should be able to create a branch here.

@UL-ChrisGlew
Copy link
Contributor Author

@gasparnagy Thanks for sorting that, I can push new branches/etc to the repository now.

From the guide it says:

You can also set the Reqnroll.VisualStudio.Package project as startup project. It is configured in a way that if you use the Start Debugging command (F5) it will automatically start the Visual Studio Experimental Instance with the debugger attached. So you can debug the full life-cycle of the extension, right from the loading. (Similarly, the "Start Without Debugging" (Ctrl+F5) is also working that simply starts Visual Studio Experimental Instance and you don't have to find it in the Start menu.)

For me, when loading the extension project, I don't see the option to debug, and get the following popup:

image
image

Normally I'd think it was something with my Visual Studio install, but I'm able to debug/launch other extensions fine.

@gasparnagy
Copy link
Contributor

That is strange. But I think you can just set the debug config to debug the devenv.exe process (from the vs install location) with the params /rootSuffix Exp. But I will check it once I am back at my desk.

@gasparnagy
Copy link
Contributor

gasparnagy commented Jun 13, 2024

I have checked and indeed the package project cannot be started by default. I might have created a launch profile earlier. Anyway: I have documented the steps to configure the launch profile: https://github.com/reqnroll/Reqnroll.VisualStudio/blob/main/CONTRIBUTION.md#debug-the-visual-studio-package

@UL-ChrisGlew I hope it will work for you as well.

@UL-ChrisGlew UL-ChrisGlew mentioned this issue Jun 13, 2024
6 tasks
@UL-ChrisGlew
Copy link
Contributor Author

@gasparnagy That worked great, thanks for writing that - hopefully it helps other contributors in the future as well 😃

@UL-ChrisGlew
Copy link
Contributor Author

@gasparnagy unrelated to this specific issue, I've noticed that the 'Find Unused Step Definitions' doesn't handle it very well if step definitions are included in a separate project.

I've tried to access the context menu option from this separate project and it throws the error below. Is this intended behavior or something that also needs to be fixed? I'm happy to look into this while I'm in this area.

image

@gasparnagy
Copy link
Contributor

@UL-ChrisGlew Thx. We were going back and forth with @clrudolphi about how the projects should be handled. So you should definitely check the discussion we had from here down. We also created a scenario to cover the cases, but it can happen that we still miss a combination.

@clrudolphi
Copy link
Contributor

IIRC we decided to not look at external assemblies because of the risks of false positives.
An external step definition assembly may be used by many different projects/solutions. A step that is not used by one project may be used by another. To mark it as 'unused' would be misleading.
We could perhaps fine-tune that with some heuristics. If the external step definition project is incorporated via Nuget for example it would be excluded but if the project containing the steps is a project within the current solution then perhaps we include it.

@UL-ChrisGlew
Copy link
Contributor Author

Ah okay, it's not as simple as a problem to solve as I first thought - it's not a dealbreaker at the moment for us but would be useful in the future.

gasparnagy pushed a commit that referenced this issue Jun 14, 2024
…ion' attribute. (#24)

* Fixing issues with finding 'Unused Step Definitions' #22

* Merge with main

* Tweaking spacing.

* Adding unit test

* Updating Changelog

* Tweaking changelog
@gasparnagy
Copy link
Contributor

I close this issue now as we will probably not be able to improve this in the close future. If anything comes up, feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants