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

HaveAnyAttributes seams to check that all exist is that how it should work? #257

Open
jnormen opened this issue Apr 22, 2024 · 7 comments
Open

Comments

@jnormen
Copy link

jnormen commented Apr 22, 2024

When I do:

HaveAnyAttributes(....)
It always require me to have the first attribute set. but when I also add a list of other attributes it force me to have them as well.

.HaveAnyAttributes(typeof(GetAttribute), typeof(PostAttribute), typeof(PutAttribute), typeof(DeleteAttribute))

 For me any sounds like if you find any of those act on them, but not it seams like it force me to have all those 4. 
 This seams odd. Why it's called Any if it not just check if any of them exist and then to the check? 
@mak638
Copy link
Collaborator

mak638 commented May 25, 2024

Hi @jnormen, thank you for your question. To me the method HaveAnyAttributes(...) works exactly as one would expect - the condition is fulfilled if any of the attributes is present for your class, interface, etc. You can find the implementation at the following line:

.Where(type => type.Attributes.Intersect(archAttributeList).Any())

You can also check the following simple Xunit example. The first test passes since the classes all have at least one of the attributes Test1 and Test2. The attribute Test3 is not used anywhere and therefore simply ignored. The second test fails since the classes in question have neither of the three attributes:

[Fact]
public void HaveAnyAttributesPassingTest()
{
    var classesWithAtLeastOneAttribute = Classes()
        .That()
        .Are(typeof(ClassWithAttribute1), typeof(ClassWithAttribute2), typeof(ClassWithBothAttributes))
        .Should()
        .HaveAnyAttributes(typeof(Test1), typeof(Test2), typeof(Test3));

    classesWithAtLeastOneAttribute.Check(Architecture);
}

[Fact]
public void HaveAnyAttributesFailingTest()
{
    var classesWithoutOneOfTheAttributes = Classes()
        .That()
        .Are(typeof(ClassWithoutAttributes), typeof(ClassWithDifferentAttribute))
        .Should()
        .HaveAnyAttributes(typeof(Test1), typeof(Test2), typeof(Test3));

    classesWithoutOneOfTheAttributes.Check(Architecture);
}

[Test1]
internal class ClassWithAttribute1 { }

[Test2]
[Test4]
internal class ClassWithAttribute2 { }

[Test1]
[Test2]
internal class ClassWithBothAttributes { }

internal class ClassWithoutAttributes { }

[Test4]
internal class ClassWithDifferentAttribute{ }

internal class Test1 : Attribute { }

internal class Test2 : Attribute { }

internal class Test3 : Attribute { }

internal class Test4 : Attribute { }

If you still observe unexpected behavior with the method feel free to add a minimal example to your question.

@mak638 mak638 closed this as not planned Won't fix, can't repro, duplicate, stale May 25, 2024
@jnormen
Copy link
Author

jnormen commented May 26, 2024

 [Fact]
    public void Interfaces_Using_Refit_Should_End_With_Api()
    {
        Interfaces()
            .That()
            .HaveAnyAttributes(typeof(GetAttribute))
            .Should()
            .HaveNameEndingWith("Client")
            .Because("All interfaces with Refit attributes should end with 'Api'")
            .Check(Architecture);
    }

GetAttribute is a Refit Atribute. When I do like this the test expect there shall be an Interface using this attribute.
If I add that I want any of the get, post, put attributes then it force me to have all 3 in my code.

So if I do:
.HaveAnyAttributes(typeof(GetAttribute),typeof(PostAttribute))

And I have a GetAttribut no Post it complains there is no Post. And if I have a Post but no Get it says the opposite.
So for me I'm forced to have all the attributes not just one of them.

for example:

 public interface ISvsClient
{
    [Post("/v1/balance-inquiry")]
    public Task<BalanceEnquiryResponse> GetBalanceEnquiryAsync(SvsBalanceEnquiryRequest svsBalanceEnquiryRequest);
}
[Fact]
    public void Interfaces_Using_Refit_Should_End_With_Api()
    {
        Interfaces()
            .That()
            .HaveAnyAttributes(typeof(GetAttribute),typeof(PostAttribute))
            .Should()
            .HaveNameEndingWith("Client")
            .Because("All interfaces with Refit attributes should end with 'Api'")
            .Check(Architecture);
    }

ArchUnitNET.Domain.Exceptions.TypeDoesNotExistInArchitecture: Type Refit.GetAttribute does not exist in provided architecture or is no attribute.

When I wrote the issue I did not check the source code, and now when I answer I haven't checked the source code either, but I see that I use Interface() not classes and are not sure if the implementation is different. I'm on the move now so just wanted to add a fast example as is.

I did not use the .Are(typeof(ClassWithAttribute1), typeof(ClassWithAttribute2), typeof(ClassWithBothAttributes))
things. maybe that something I need to add?

@fgather
Copy link
Contributor

fgather commented Sep 3, 2024

Hi @jnormen,

could you quickly share with us how you load your classes? Are you sure that the Refit Attributes are correctly loaded and part of the analysed architecture?

@fgather fgather reopened this Sep 3, 2024
@jnormen
Copy link
Author

jnormen commented Sep 3, 2024

I use Refit.
So it's interfaces.

You put attributes on them like this:

public interface IHelloClient { [Get("/hello/{greet}")] public Task<Response<string>> GetHelloAsync(string greet, CancellationToken cancellationToken = default); }

then I do like this:
` private static readonly Architecture Architecture =
new ArchLoader().LoadAssemblies(typeof(Program).Assembly).Build();

[Fact]
public void Interfaces_Using_Refit_Should_End_With_Client()
{
    Interfaces()
        .That()
        .HaveAnyAttributes(typeof(GetAttribute))
        .Should()
        .HaveNameEndingWith("Client")
        .Because("All interfaces with Refit attributes should end with 'Client'")
        .Check(Architecture);`

This works, but I want to do like this:
.HaveAnyAttributes(typeof(GetAttribute),typeof(PostAttribute),typeof(PutAttribute), typeof(DeleteAttribute) )

In this case it force me to have all set. Any for me is like, if any of them exist, then it should act.
But in my case I'm forced to have all of them set.

I want to use it as a generic verification. Not all clients will have all those attributes set.
I tried Or too but same problem.

So what I want to do is: If any of those attributes exist then it shall act and check that the name end with Client.

If that's not how any works, then the naming is strange and the bug it more typo :)

@alexanderlinne
Copy link
Collaborator

alexanderlinne commented Sep 9, 2024

Hi @jnormen,

since the attributes from Refit are added to the methods of the interface, this has to be taken into account when writing the test. So this test would check, that any methods with any of these attributes are defined in interfaces ending with Api:

MethodMembers()
    .That()
    .HaveAnyAttributes(
        typeof(GetAttribute),
        typeof(PostAttribute),
        typeof(PutAttribute),
        typeof(DeleteAttribute)
    )            
    .And()
    .AreDeclaredIn(Interfaces())
    .Should()
    .BeDeclaredIn("^.*Api$", true)
    .Because("All interfaces with Refit attributes should end with 'Api'")
    .Check(Architecture);

I could not reproduce the TypeDoesNotExistInArchitecture exception. In a simple example with one classlib and one test assembly this worked without any problems. We would need an example (zip file or public github repository) that reproduces the issue to further investigate this. -> We did now also find an example where we get the exception. We will need to have a closer look as to why this happens.

@jnormen
Copy link
Author

jnormen commented Sep 10, 2024

I don't understand how you get this to work... I still get the same error.
For me it does not act on ANY it act on all. If there is a Get no problem, if I add any and Post it gives me this error:

ArchUnitNET.Domain.Exceptions.TypeDoesNotExistInArchitecture: Type Refit.PostAttribute does not exist in provided architecture or is no attribute.

If I add a Post method in refit interface it complains that a Put does not exist.

ANY for me is if there is a get then act, if there is a post, then act, if there is a delete than act.
If none of them exist complain. But the ANY still force me to add all of them not one of them.

For exmaple:

I have a Refit Interface with a get set when I run this code:

MethodMembers() .That() .HaveAnyAttributes( typeof(GetAttribute), typeof(PostAttribute), typeof(PutAttribute), typeof(DeleteAttribute) ) .And() .AreDeclaredIn(Interfaces()) .Should() .BeDeclaredIn("^.*Api$", true) .Because("All interfaces with Refit attributes should end with 'Api'") .Check(Architecture);

I get
ArchUnitNET.Domain.Exceptions.TypeDoesNotExistInArchitecture: Type Refit.PostAttribute does not exist in provided architecture or is no attribute.

If I also add a Post attribute on a method in my interface it complains that a Put does not exist. 

I expect a ANY is iF any of them exist then act...  If there is none of them = error. if one of them exist, then pass. 

@alexanderlinne
Copy link
Collaborator

Since we do not have a full example it is hard to reproduce your exact issue on our side. Could you please run the attached sample project, for me this neither throws any exception nor does the test fail.

LibaryTest.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants