-
-
Notifications
You must be signed in to change notification settings - Fork 544
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
Add feature to check if an XElement or XAttribute is absent within the XDocument #2589
Comments
Sure, makes sense. What do you think @jnyrup ? |
It certainly makes sense to have more negated assertions for XML types 👍 You suggest adding an overload of We currently have
and
In order to have a consistent API, it would be be nice to add the proposed methods on all relevant classes. |
Hey @jnyrup and @dennisdoomen Thanks for the reply, should I implement the changes according to your guidelines and create another PR? (I mean, I really need this extension because I have a dirty workaround for my problem (I've added a BG Sergej |
Everything that is in this proposal should be delivered as a consistent set of changes. And to prevent contributors from abandoning the work after the first PR, we need to insist on contributors to provide a PR that covers everything that is needed to maintain that consistency. Hope you understand. |
It took me a couple of seconds to realize that In that case, we need:
To be clear, you can only add the
|
@jnyrup @skukshaus any thoughts? |
Sorry for the delay, Dennis, |
We currently have To avoid scope creeping, I suggest we focus on public class XDocumentAssertions
{
public AndConstraint<XDocumentAssertions> NotHaveElement(string unexpected, string because = "", params object[] becauseArgs)
public AndConstraint<XDocumentAssertions> NotHaveElement(XName unexpected, string because = "", params object[] becauseArgs)
}
public class XElementAssertions
{
public AndConstraint<XElementAssertions> NotHaveElement(string unexpected, string because = "", params object[] becauseArgs)
public AndConstraint<XElementAssertions> NotHaveElement(XName unexpected, string because = "", params object[] becauseArgs)
}
public class XmlElementAssertions
{
public AndConstraint<XmlElementAssertions> NotHaveElement(string unexpectedName, string because = "", params object[] becauseArgs)
} |
I'd rather use this opportunity to correct the situation. |
Hey, I need this also - willing to help if you need an extra pair of hands. |
@leus as we're discussing several methods, which one(s) are you looking for? |
Then I guess we should have:
|
I think the last one is a bit ambiguous. I would opt for |
Yes, on
ChatGPT believes that the behavior suggested by @jnyrup makes more sense. I tend to agree. |
@leus I'll ping you when we have settled on the API. |
Sure - in the meantime I'm using |
If we let Asserting that some has an attribute but not with a given value would be To avoid that confusion, we could e.g. choose to not expose either but let the developer write the unambigious: foo.Should().HaveAttribute(bar)
.Which.Value.Should().NotBe(baz);
Why? |
Although I've learned something new again (thank you 😜), such formal definitions don't mean much for must developers. In the end it's about intuitivity.
I like that.
To reduce the scope of this PR and because |
How could we check if an attribute is missing or set to I would expect XElement.Parse("<root attr=\"\" />").Should().NotHaveAttribute("attr");
// -> succeeds So we would need an extension like |
In #2321 we changed our code to behave more logically, because our intuition failed to imagine how developers would use the API. I agree that logic is not always immediate intuitive and I don't mean to force all developers to know De Morgan's law by heart, but I would conjecture that looking back at the past thousands of years that logic has a better track record than intuition. Thinking more about it, here's an example of where I would say the logically correct version of Should fail: var subject = XElement.Parse("<root error=\"myErrorType\" />");
subject.Should().NotHaveAttributeWithValue("error", "myErrorType"); Both of these should pass because they either have another error value or no error value at all. var subject = XElement.Parse("<root error=\"someOthererrorType\" />");
subject.Should().NotHaveAttributeWithValue("error", "errorType");
var subject = XElement.Parse("<root />");
subject.Should().NotHaveAttributeWithValue("error", "errorType");
Then I suggest that after approving an API for |
For me that has been almost 30 years ago. I'm already happy that I sometimes still remember what my wife said yesterday ;-) |
Actually, I would expect this one to have a single parameter and mean that whatever attribute the element has, it should not have that value. |
That's a completely different situation. And I don't believe an attribute with an empty string is the same. In fact, it isn't. |
Which brings us back to the (hopefully) final proposal:
Plus overloads where it takes an Agreed @jnyrup? |
In your final proposal the handling of "null or empty attribute" isn't covered, see #2589 (comment) |
It doesn't and it won't. As I said here, I don't see that as a valid argument. |
I agree with the proposed methods in #2589 (comment) |
It's not always the same, but it can be. Like for strings, where |
Can I throw in another thought, or is it too late? In #2321 we changed the behavior to not check two things in one method. |
We always welcome input (until we express otherwise).
We currently have <root>
<child>a</child>
<child>b</child>
</root> Am I misreading what you're concerns are? |
Does this #2589 (comment) approved API include both ( If so, should this go into a separate PR, because this PR is pretty big already... I organized the commits so that it should be easy to cherry-pick to a different PR... |
Yes, because without it, we would end up with an inconsistent experience. |
Background and motivation
Currently, there is an implementation to check whenever an Attribute or Element is within the Document: https://fluentassertions.com/xml/
However, sometimes it is necessary to check if those attributes are absent. Currently, there is no possibility to do this. Maybe it would be possible to use the HaveElement() Method and catch the Exception, but this feels dirty.
I think it could also be a good idea, to add a constraint if the attribute/element does not have a specific value
API Proposal
API Usage
Alternative Designs
No response
Risks
No response
Are you willing to help with a proof-of-concept (as PR in that or a separate repo) first and as pull-request later on?
Yes, please assign this issue to me.
The text was updated successfully, but these errors were encountered: