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.

Sign up for GitHub

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

Jump to bottom

Feature suggestion: Freeze files and stored.rules validation / sanity check #1264

Open
keiki85 opened this issue Mar 12, 2024 · 4 comments · May be fixed by #1405 or #1407
Open

Feature suggestion: Freeze files and stored.rules validation / sanity check #1264

keiki85 opened this issue Mar 12, 2024 · 4 comments · May be fixed by #1405 or #1407
Labels
enhancement good start to contributing help wanted topic:library

Comments

@keiki85
Copy link

keiki85 commented Mar 12, 2024 •
edited
Loading

Hi ArchUnit Team,

First of you do a great job here. ArchUnit is great.

In our project we currently have >70 freeze files in the stored.rules mentioned.

Digging deeper I noticed several things, which I am concerned about.

  1. The files referenced might not exist anymore, but the reference in stored.rules is
  2. When freeze files become empty you are not being advised to remove the freezing from your test.
  3. Freeze files might still be in the archunit folder, but not referenced anymore
  4. Tests were removed without removing the stored.rules line or freeze file
  5. stored.rules are not sorted. To make it better human readable it would be nice to have it sorted alphabetically.

Of course in the end it is our job to keep our source code in check and clean up correctly etc.

Yet I wonder how ArchUnit itself can help to have a sanity check of its freeze files and stored.rules?

What is your opinion on this?
@codecholeric
Copy link
Collaborator

Sorry for the late reply, I think it would be a nice addition, if anybody wants to give this a shot I would support it! In the past we've already sorted the actual violations alphabetically to ease human browsing and diffs if I remember correctly.

@maxxkia
Copy link

maxxkia commented Jan 18, 2025

Hello 👋
I would like to work on this!

maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 18, 2025
@maxxkia
raise error for empty violation store
741bf11 
Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
@maxxkia maxxkia linked a pull request Jan 18, 2025 that will close this issue
Open
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 18, 2025
@maxxkia
raise error for empty violation store
67c54aa 
Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
raise error for empty violation store
3401094 
Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
raise error for empty violation store
04445a0 
delete empty rule violation file

Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
raise error for empty violation store
5e2af43 
warn on empty rule violation file

Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
raise error for empty violation store
a833065 
warn on empty rule violation file
delete empty rule violation file

Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
raise error for empty violation store
90c5338 
warn on empty rule violation file
delete empty rule violation file

Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
@maxxkia
Copy link

maxxkia commented Jan 19, 2025 •
edited
Loading

Hi @codecholeric

I have created #1405 which solves parts of the features requested above, request no. 2 to be specific.

  1. When freeze files become empty you are not being advised to remove the freezing from your test.

If those changes make sense to you I can proceed and update the docs in the same PR.

@maxxkia
Copy link

maxxkia commented Jan 19, 2025

I can work on the following feature requests in another PR

  1. The files referenced might not exist anymore, but the reference in stored.rules is
  2. Freeze files might still be in the archunit folder, but not referenced anymore
  3. stored.rules are not sorted. To make it better human readable it would be nice to have it sorted alphabetically.

The 4th request

  1. Tests were removed without removing the stored.rules line or freeze file
    is more complicated because it requires taking an action by considering the whole test suite. At this point, I don't know if it is possible to achieve that.

maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
raise error for empty violation store and delete empty rule file
60de7ab 
These changes introduce two new features for FreezingRule default store
* Raise error when a freezing rule has zero violations. This can be enabled by setting the property `default.warnEmptyRuleViolation=true`. For backward compatibility it is disabled by default.
* Skip rule violation file creation or delete if it already exists, when there are zero violations. This can be enabled by setting the property `default.deleteEmptyRuleViolation=true`, it is disabled by default.

Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
cleanup obsolete frozen rules
39fe12b 
Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
@maxxkia maxxkia linked a pull request Jan 19, 2025 that will close this issue
Draft
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
These changes bring 2 features
7f8a0bf 
* clean up of frozen rules in `stored.rules` where the corresponding file does not exist in the store directory
* clean up of files in store directory which are not referenced in `stored.rules` file
Both of the above operations are enabled using `default.allowStoreUpdate` property. If `default.allowStoreUpdate=false` and obsolete entries or files are found by either of the above operations, the operation fails with an `StoreUpdateFailedException`.

Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
maxxkia added a commit to maxxkia/ArchUnit that referenced this issue Jan 19, 2025
@maxxkia
These changes bring 2 features
099f636 
* clean up of frozen rules in `stored.rules` where the corresponding file does not exist in the store directory
* clean up of files in store directory which are not referenced in `stored.rules` file
Both of the above operations are enabled using `default.allowStoreUpdate` property. If `default.allowStoreUpdate=false` and obsolete entries or files are found by either of the above operations, the operation fails with an `StoreUpdateFailedException`.

Signed-off-by: Masoud Kiaeeha <[email protected]>

Resolves TNG#1264
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement good start to contributing help wanted topic:library
Projects
None yet
Milestone
No milestone
3 participants
@codecholeric @maxxkia @keiki85