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

Task input to configure how metrics count "Test" code #461

Open
cjprecord opened this issue Feb 8, 2024 · 4 comments
Open

Task input to configure how metrics count "Test" code #461

cjprecord opened this issue Feb 8, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@cjprecord
Copy link

Summary

Currently the task determines test code metric by any file that has the word Test in its name

(/.((T|t)est|TEST)./.test(entry.fileName)

A lot of repos we work with have files with Test in name that are not test code. Would be nice if task had configuration option to accept some file glob patterns or similar to filter test code files. For example a lot of times our projects use a namespace / folder where tests are defined.

Example:
repo

  • SomeAssembly
    • SystemTest.cs
    • SystemTestModule.cs
    • TestModuleAdapter.cs
  • AutomatedTests
    • SomeAssemblyTest
      • TestSystemTest.cs
      • ....

Would like to set a file glob pattern to only count changes to files with "AutomatedTests" in their name for example as the code in "SomeAssembly" is not test code but has "Test" in the name. All the test code is contained under folder with AutomateTests in the name.

Rationale

Will allow for more accurate and meaningful information in the metrics provided back to the PR to use for review or gating purposes.

@cjprecord cjprecord added the enhancement New feature or request label Feb 8, 2024
@muiriswoulfe
Copy link
Member

Thanks @cjprecord for using PR Metrics and for filing your suggestion.

This is something that has been considered in the past, but to reduce the complexity – especially given the possibility of interference between multiple sets of globs and the need to define clear precedence rules – it was decided against adding this.

The other rationale is that it encourages the proper naming of code – i.e., that you don't put test in product code for instance.

To help determine whether we proceed with changing this and the proper level of prioritisation, I'd like to understand:

  • Why you have Test in the names of non-test files? If I saw SystemTest.cs, I would assume it was testing the system and therefore was test code.
  • If this is some form of testing that you don't want included, would it make sense to exclude this using the excluded files functionality?

@cjprecord
Copy link
Author

Hello,

Use of an optional regular expression string would work for this as well and probably be less complicated to existing functionality.

Depending on industry or codebase use there are a lot of cases where Test may appear in a file name that is not a test case. A big one for us is we have a lot of code that is used for mechanical or electrical control systems. Lots of components are used for 'self' tests, diagnostics, or literally running tests on the target systems or other diagnostic purposes and use of Test in the class/file names is very sensible and common.

Where these 'tests' are not tests for the code base being developed but tests that are performed on a target system, purpose of report on 'test' code should be for the code quality of the code being developed and not inflated by the 'tests' that the production code runs on other systems.

An exclude pattern could possibly work but would be more complicated to use in the projects we have. Typically we use a namespace / folder path structure to organize software tests code from production code, common example pattern is like this where all software test code is under an AutomatedTests folder

ModuleAlpha
AutomatedTests
ModuleAUT
ModuleBUT
ModuleA
ModuleB

@muiriswoulfe
Copy link
Member

Thanks @cjprecord for the justification. I now believe it's viable for PR Metrics to support something like that.

The bad news is that we won't be able to prioritise this work anytime soon, but I am happy to support you implementing it if it is something you would be willing to take on.

As for the regular expression/glob possibilities, I really favour using a glob to keep the existing checks aligned. I believe switching between the two for different parameters will make the extension more difficult to consume.

@cjprecord
Copy link
Author

Thanks for the consideration.

If I get some time, will try to look at submitting a patch

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

2 participants