search

TNG / ArchUnitNET

A C# architecture test library to specify and assert architecture rules in C# for automated testing.
Apache License 2.0
826 stars 55 forks source link

Question about ResideInAssembly #245

Open pawlos opened 3 months ago

pawlos commented 3 months ago

I've been playing with the library and encountered an issue with the above mentioned method.

The signature of it is

ResideInAssembly(string pattern, bool useRegularExpressions = false)

So my assumption was that it should match an assembly by name, with either using a regular expressions or w/o, just by simply comparing parts of the name.

To my surprise that later option didn't work no matter what parts of assembly name I would give.

Today, I dig through the code and discovered that in case of non-regular expression version the match is by the full assembly name. It was a bit counter intuitive for me as the pattern variable name would suggest that it's should match less restrictively (maybe with StartsWith?). The pattern parameter name makes sens for RegEx option but not (for me) for the non-regex one.

At least a /// <remark> would be nice to be there to indicate that in case of non-regex one it should be a full assembly name. If that's ok I could create PR with adding an info or maybe it's ok to somehow redesign this to me more clear what's the intent of the method? Or maybe it should actually match FullName of an assembly using a .StartsWith?

alexanderlinne commented 2 months ago

Hi @pawlos, if you'd like to create a PR with a documentation comment we'd welcome that.

We are currently also discussing a rework of these APIs to use separate functions like ResideInAssemblyWithName and ResideInAssemblyWithNameMatching instead of a bool parameter. Another option would be ResideInAssembly(Assemblies().That().HaveNameStartingWith("System")). Of course this would be a larger change so this will probably not be implemented soon.

pawlos commented 2 months ago

hello @alexanderlinne

Thanks for your reply. Restructuring the API would be the best but I do get that it's the larger change. I've pushed a small PR to add this extra info. Please let me know if I can improve it in any way.