Fix ResourceQueryFilter constraints and add test cases - #1361
#1361Fix ResourceQueryFilter constraints and add test cases#1361kumarnitin17 wants to merge 2 commits intodevPHOENIXCONTACT/MORYX-Framework:devfrom fix/resource-resourcequeryfilterPHOENIXCONTACT/MORYX-Framework:fix/resource-resourcequeryfilterCopy head branch name to clipboard
Conversation
| <PropertyGroup> | ||
| <TargetFramework>net10.0</TargetFramework> | ||
| <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath> | ||
| <DebugType>full</DebugType> |
There was a problem hiding this comment.
You want to add IsPackable - false, Nullable enable and ImplicitUsings enable here. The last to flags are added to all new probjects at least and we should respect the nullable warnings here as well and handle them meaningfully 😊
There was a problem hiding this comment.
Added the standard project settings:
- IsPackable=false
- Nullable=enable
- ImplicitUsings=enable
This aligns the test project with the configuration used in the other projects and enables nullable reference type checks.
There was a problem hiding this comment.
We forgot <IsTestProject>true</IsTestProject> unfortunately
There was a problem hiding this comment.
Can you check this file again, looks like you got the line ending issue again
There was a problem hiding this comment.
The MORYX-Framework .slnx modification was caused by a line-ending issue. I have reverted the unintended change and updated the PR.
There was a problem hiding this comment.
Did you copy these classes? 🙈 If you want to reuse them please move them to Moryx.AbstractionLayer.TestTools there is already a tab for resources. Also remove them from the other test project, we don't want them duplicated
There was a problem hiding this comment.
The classes were duplicated. I moved the shared resource test classes to Moryx.AbstractionLayer.TestTools.Resources and removed the duplicate copies from Moryx.AbstractionLayer.Resources.Endpoints.Tests. The tests now use the shared implementations from TestTools.
| } | ||
|
|
||
| [Test(Description = "Match returns false when reference collection is empty and constraint is NotEmpty")] | ||
| public void Match_WithEmptyReferenceCollection_AndNotEmptyConstraint_ReturnsFalse() |
There was a problem hiding this comment.
Being picky here, please remove the middle _ as it conflicts with the microsoft naming guideline 🙂
There was a problem hiding this comment.
Removed the underscores from the test method name to align with the Microsoft naming guidelines.
There was a problem hiding this comment.
I only meant the middle _ in the name. Please check the guideline yourselves, understand it and apply the naming convention correctly 😊
6f63d2a to
7bc9635
Compare
|
|
||
| return Accepted(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Instead of making this internal, it should be tested as a blackbox, there is no need for the production code to make it internal, just for tests is ab bad pattern(i know, we have a lot of them). Its now a good sample that You test implementation instead of behavior. Even its internal, you open parts of the assembly, only the public api should be tested.
Sometimes there are reasons why internal is okay because when you want to test full components which are e.g. in the di-infrastructure isolated behind an api.
| @@ -1,186 +1,187 @@ | ||
| <Solution> |
There was a problem hiding this comment.
What happened here? Line endings?
Summary
Unit Test Cases