Fix: Duplication in Puml errormessages; #122

ZDI-StephanJoneleit · 2021-09-27T08:33:17Z

Assigned to Issue #121

We fixed the problem with the errormessages in puml by adding a cache like in the non-puml versions.
Duplication will be now filtered.

Signed-off-by: Stephan Joneleit <[email protected]>

fgather · 2021-09-27T22:27:12Z

Hi, @ZDI-StephanJoneleit ,

thanks for your contribution! I wonder if you could provide a test-case for this? Maybe @brandhuf could help out here?

ZDI-StephanJoneleit · 2021-09-28T06:38:53Z

Hi, @ZDI-StephanJoneleit ,

thanks for your contribution! I wonder if you could provide a test-case for this? Maybe @brandhuf could help out here?

TestCase_ArchUniteNet_Issue121_PullRequest122.txt

We hope the test case will help you.

Remarks:
Issue, Fix and Docs are provided from one of our students.
He's doing his best to help you out.

Kind Regards,
your ZDI Team and Stephan Joneleit

fgather · 2021-09-28T21:13:31Z

ArchUnitNET/Fluent/Syntax/Elements/Types/TypeConditionsDefinition.cs

+
+                //Prevent failDescriptions like "does depend on X and does depend on X and does depend on Y and does depend on Y
+                var failDescriptionCache = new List<string>();
+
                foreach (var dependency in ruleType.GetTypeDependencies())


I wonder:
Instead of filtering afterwards, wouldn't Distinct() operation on the dependencies list or rather a DistinctBy on the dependencies with regard to the dependency.FullName property be easier to read?
DistinctBy in LINQ can be emulated with: dependency.GroupBy(p => p.FullName).Select(g => g.First())

Could you convert the very good testcase you provided (thanks again!) into an automated test? If you don't find the time I can do this as well, but it might take a while.

I'm happy to see that ArchUnitNet made its way to Zeiss :)

Distinct is now implemented and tested.
The result stays the same but is now easier to read.

The automated test will be added later.

Also we want to thank You (all) for this great Open Source Project.
It does help us a lot for our .NET/Java Health Checks.
Especially the PlantUML Support is a huge help for larger projects.

… instead of during iteration; Signed-off-by: Stephan Joneleit <[email protected]>

Signed-off-by: Stephan Joneleit <[email protected]>

Remarks: I'm sorry but I was only able to run the test with an empty global.json file; Signed-off-by: Stephan Joneleit <[email protected]>

ZDI-StephanJoneleit · 2021-09-29T09:55:22Z

ArchUnit.sln

@@ -1,7 +1,7 @@

 Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio 15
-VisualStudioVersion = 15.0.26430.16
+# Visual Studio Version 16


I'm Sorry, it looks like VS updated the Solution File on it's own.
It was not shown in the commits.

I'm sorry, but it didn't passed the automatic tests.
It might be due the changes in the solution file VS made on it's own (which was not shown in the commits)
and I had problems with the global.json file, which I had to clear so that the tests run.

I try my best to fix these problems.

Kind Regards:

Joneleit, Stephan

ZDI-StephanJoneleit · 2021-09-29T10:00:51Z

That's how the test result would look like if the errormessage contains duplications:

And that's how it should look like (successfull due no duplications):

Fix: Unsupported SDK in global.json -> SDK Version 5.0.203 cannot be downloaded on the official dotnet download page (minimun 5.0.202 which is unfortunally not 5.0.203), that's why I included rollForward; Signed-off-by: Stephan Joneleit <[email protected]>

ZDI-StephanJoneleit · 2021-09-29T12:03:15Z

global.json

@@ -1,5 +1,6 @@
 {
  "sdk": {
-    "version": "5.0.203"
+    "version": "5.0.203",


I needed to change something in the global.json file.

Unfortunally, you only can download SDK Version 5.0.202 or 5.0.3xx from the official .NET Website.
Because my SDKs MUST match 5.0.203 (which you cannot download (maybe I only didn't found it) I use SDK 5.0.202 (which is lower) and SDK 5.0.3xx (which is the next feature version).

I do recommend to set version to 5.0.202 (which you still can download) and use an own rollForward (for example latestMajor/Minor (in the future when new features are added) (and on my machine was per default only installed the latest (SDK) version).

I found the SDK 5.0.203 after a little deeper search.

…n the .NET Website because the Bot "Travis CI" failed (maybe due different SDK Versions); Signed-off-by: Stephan Joneleit <[email protected]>

Signed-off-by: Stephan Joneleit <[email protected]>

ZDI-StephanJoneleit · 2021-09-29T12:24:27Z

For some reasons the Update still does not pass throu Travis CI.
So you might need to do some tests manually to verify that the code is working.

I'll try my best to help you as good as I can, so feel free to contact me again.

Kind Regards,

Joneleit, Stephan; (Student) from Zeiss Digital Innovation

ZDI-StephanJoneleit · 2021-10-01T06:11:58Z

For some reasons the Update still does not pass throu Travis CI. So you might need to do some tests manually to verify that the code is working.

I'll try my best to help you as good as I can, so feel free to contact me again.

Kind Regards,

Joneleit, Stephan; (Student) from Zeiss Digital Innovation

@fgather , do you have an Idea why it failed?

Could it be due the test I wrote and included?
Also it could be due the path to the puml (normally it should create the correct path (format) based on the OS (path format)).

YeGaSoft · 2021-10-01T18:02:37Z

ArchUnitNet.MSTest.CheckErrormessages/PlantUMLErrormessagesCheck.cs

+
+            var groups = splittedMessages.GroupBy(x => x.Trim().Trim('\t'));
+            var duplications = groups.Where(x => (x.Count() > 1 
+            && !String.IsNullOrWhiteSpace(x.FirstOrDefault()))


Switch && and || to ensure Empty strings will be ignored.

!String.IsNullOrWhitespace(x.FirstOrDefaul) && (x.Count() > 1 || firstError.Contains(x.First()))

This would protect from errors due Empty or Whitespace strings (depending on the string inside FirstError).

Signed-off-by: Stephan Joneleit <[email protected]>

ZDI-StephanJoneleit · 2021-10-04T09:21:41Z

For some reasons the Update still does not pass throu Travis CI. So you might need to do some tests manually to verify that the code is working.
I'll try my best to help you as good as I can, so feel free to contact me again.
Kind Regards,

Joneleit, Stephan; (Student) from Zeiss Digital Innovation

@fgather , do you have an Idea why it failed?

Could it be due the test I wrote and included? Also it could be due the path to the puml (normally it should create the correct path (format) based on the OS (path format)).

Solved(?)

…rchUnitNET should be in there) copy the classes for the PlantUML diagram to the TestAssembly (ExampleTests should only be used for examples and not for testing purposes) Signed-off-by: Fritz Brandhuber <[email protected]>

Signed-off-by: Fritz Brandhuber <[email protected]>

…to 121_FixDuplicationsInPumlErrormessages # Conflicts: # ArchUnitNet.MSTest.CheckErrormessages/PlantUMLErrormessagesCheck.cs

brandhuf · 2021-10-04T14:39:47Z

@ZDI-StephanJoneleit I created a PR to your branch with some improvements.

Mainly I moved the TestClass to the ArchUnitNETTests assembly, where all tests for ArchUnitNET should be. I also copied the classes for the PlantUML file to the TestAssembly, because ExampleTest should only contain examples and should not be used for testing. I also simplified the test a bit, because it was quite complicated before.

Can you please accept it? Afterwards we can merge this into master.

…sages improvements to 121 fix duplications in puml errormessages

Fix: Duplication in Puml errormessages;

7825108

Signed-off-by: Stephan Joneleit <[email protected]>

ZDI-StephanJoneleit force-pushed the 121_FixDuplicationsInPumlErrormessages branch from 21e5959 to 7825108 Compare September 27, 2021 08:44

fgather reviewed Sep 28, 2021

View reviewed changes

Update: Implemented 'Distict' to filter duplication before interation…

a7846b0

… instead of during iteration; Signed-off-by: Stephan Joneleit <[email protected]>

ZDI-StephanJoneleit force-pushed the 121_FixDuplicationsInPumlErrormessages branch from 1b7dd47 to a7846b0 Compare September 29, 2021 06:17

Add: Test PUML;

3db7197

Signed-off-by: Stephan Joneleit <[email protected]>

ZDI-StephanJoneleit force-pushed the 121_FixDuplicationsInPumlErrormessages branch from a2deaea to 3db7197 Compare September 29, 2021 09:51

Add: Test to check for duplications in errormessages;

8bc0147

Remarks: I'm sorry but I was only able to run the test with an empty global.json file; Signed-off-by: Stephan Joneleit <[email protected]>

ZDI-StephanJoneleit force-pushed the 121_FixDuplicationsInPumlErrormessages branch from 1c3c5ac to 8bc0147 Compare September 29, 2021 09:53

ZDI-StephanJoneleit commented Sep 29, 2021

View reviewed changes

ZDI-StephanJoneleit added 2 commits September 29, 2021 14:06

Updated: Changed version to nexr lower version that is downloadable o…

c958bc4

…n the .NET Website because the Bot "Travis CI" failed (maybe due different SDK Versions); Signed-off-by: Stephan Joneleit <[email protected]>

Update: global.json (again) -> found SDK Version 5.0.203;

f60ddd5

Signed-off-by: Stephan Joneleit <[email protected]>

YeGaSoft reviewed Oct 1, 2021

View reviewed changes

Update: Improved Where Condition;

6594878

Signed-off-by: Stephan Joneleit <[email protected]>

brandhuf added 4 commits October 4, 2021 15:50

simplify the test for better readability

32a1994

Signed-off-by: Fritz Brandhuber <[email protected]>

remove the MSTest csproj file and revert some small changes

7f9d306

Signed-off-by: Fritz Brandhuber <[email protected]>

Merge branch '121_FixDuplicationsInPumlErrormessages_improvements' in…

fea565c

…to 121_FixDuplicationsInPumlErrormessages # Conflicts: # ArchUnitNet.MSTest.CheckErrormessages/PlantUMLErrormessagesCheck.cs

Merge pull request #1 from brandhuf/121_FixDuplicationsInPumlErrormes…

895269c

…sages improvements to 121 fix duplications in puml errormessages

fgather merged commit 2db16b7 into TNG:main Oct 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Duplication in Puml errormessages; #122

Fix: Duplication in Puml errormessages; #122

ZDI-StephanJoneleit commented Sep 27, 2021

fgather commented Sep 27, 2021

ZDI-StephanJoneleit commented Sep 28, 2021

fgather Sep 28, 2021

ZDI-StephanJoneleit Sep 29, 2021

ZDI-StephanJoneleit Sep 29, 2021

ZDI-StephanJoneleit Sep 29, 2021

ZDI-StephanJoneleit commented Sep 29, 2021

ZDI-StephanJoneleit Sep 29, 2021

ZDI-StephanJoneleit Sep 29, 2021

ZDI-StephanJoneleit commented Sep 29, 2021

ZDI-StephanJoneleit commented Oct 1, 2021

YeGaSoft Oct 1, 2021

ZDI-StephanJoneleit Oct 4, 2021

ZDI-StephanJoneleit commented Oct 4, 2021

brandhuf commented Oct 4, 2021

Fix: Duplication in Puml errormessages; #122

Fix: Duplication in Puml errormessages; #122

Conversation

ZDI-StephanJoneleit commented Sep 27, 2021

fgather commented Sep 27, 2021

ZDI-StephanJoneleit commented Sep 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZDI-StephanJoneleit commented Sep 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZDI-StephanJoneleit commented Sep 29, 2021

ZDI-StephanJoneleit commented Oct 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZDI-StephanJoneleit commented Oct 4, 2021

brandhuf commented Oct 4, 2021