Skip to content

And constraint on Has.Member throws #2411

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

Closed
ChrisFewtrell opened this issue Sep 4, 2017 · 7 comments · Fixed by #2447
Closed

And constraint on Has.Member throws #2411

ChrisFewtrell opened this issue Sep 4, 2017 · 7 comments · Fixed by #2447
Assignees
Labels
Milestone

Comments

@ChrisFewtrell
Copy link

An example

var donkey = new object();
var mule = new object();
Assert.That(new[] { donkey }, Has.Member(donkey).And.No.Member(mule));

Fails with this exception in 3.8.1

System.ArgumentException : The actual value must be an IEnumerable
Parameter name: actual
   at NUnit.Framework.Constraints.SomeItemsConstraint.ApplyTo[TActual](TActual actual) in D:\git\nunit\src\NUnitFramework\framework\Constraints\SomeItemsConstraint.cs:line 64
   at NUnit.Framework.Constraints.NotConstraint.ApplyTo[TActual](TActual actual) in D:\git\nunit\src\NUnitFramework\framework\Constraints\NotConstraint.cs:line 48
   at NUnit.Framework.Constraints.AndConstraint.ApplyTo[TActual](TActual actual) in D:\git\nunit\src\NUnitFramework\framework\Constraints\AndConstraint.cs:line 64
   at NUnit.Framework.Constraints.SomeItemsConstraint.ApplyTo[TActual](TActual actual) in D:\git\nunit\src\NUnitFramework\framework\Constraints\SomeItemsConstraint.cs:line 65
   at NUnit.Framework.Assert.That[TActual](TActual actual, IResolveConstraint expression, String message, Object[] args) in D:\git\nunit\src\NUnitFramework\framework\Assert.That.cs:line 245
   at NUnit.Framework.Assert.That[TActual](TActual actual, IResolveConstraint expression) in D:\git\nunit\src\NUnitFramework\framework\Assert.That.cs:line 228
   at Granta.Services.FunctionalData.Tests.ParameterValueExtraction.ParameterSingleValuesExtractorDatabaseTests.MethodUsingObjectsFailsInDifferentManner() in D:\git\servicelayer\FunctionalDataTests\ParameterValueExtraction\ParameterSingleValuesExtractorDatabaseTests.cs:line 59

This is a regression from 3.6 and I suspect was introduced by #2239.
The right hand constraint of the And is getting evaluated against each item of the array instead of against the enumerable. I have tried stepping through and figuring out where things go wrong but without success.

I presume I am using the API correctly - I expected the assertion to check that the array contained donkey and did not contain mule. If I am wrong, I would appreciate some guidance on how to chain these constraints using the fluent API - or should one write 2 separate assertions?

@mikkelbu
Copy link
Member

mikkelbu commented Sep 5, 2017

Before implementing a solution to this I think we should have tests that cover all the functionality of CollectionContainsConstraint - as it were in version 3.7 - and then use these tests to ensure that we have kept all the behaviour. I'll try to look into writing these tests tonight (if I have time). The tests should also cover #2412.

@mikkelbu
Copy link
Member

mikkelbu commented Sep 5, 2017

I've tried to go through all the changes done in #2239 and write tests for each change. All the tests pass in version 3.7.1, but in 3.8.1:

  • 5 does not compile (due to 'SomeItemsConstraint' does not contain a definition for 'Using'), and
  • 3 fails with System.ArgumentException : The actual value must be an IEnumerable

https://gist.github.com/mikkelbu/370fe6a735c41906dca1fad28282f19e

@mikkelbu mikkelbu added the is:bug label Sep 5, 2017
@mikkelbu mikkelbu self-assigned this Sep 9, 2017
mikkelbu added a commit that referenced this issue Sep 17, 2017
Unfortunately need to implement a lot of Using
overloads in SomeItemsConstraint and a dynamic check.

Fixes #2411 and fixes #2412
@mikkelbu mikkelbu added this to the 3.8.2 milestone Sep 19, 2017
@mikkelbu
Copy link
Member

@rprouse I've added this issue to the 3.8.2 milestone.

@rprouse rprouse modified the milestones: 3.8.2, 3.9 Nov 3, 2017
@Snaaio
Copy link

Snaaio commented Dec 4, 2017

I am currently using nunit.framework v3.9.0.0 and still have problems when using a Has.One.Member(...) constraint:
class TestElement { }

var element = new TestElement();
Assert.That(new System.Collections.Generic.List<TestElement> { element }, Has.One.Member(element));

Test fails with the following error:

Message: System.ArgumentException : Expected: IEnumerable But was: TestElement
Parameter name: actual

Does this mean this issue is not completely fixed, is it a new issue or am I doing something wrong?

@ChrisMaddock
Copy link
Member

@Snaaio - would you file a new issue for this please? 🙂 I'm not entirely sure how the syntax should work here off the top of my head - is this a regression from a previous version of the framework?

@CharliePoole
Copy link
Member

I'm suspicious of the syntax Has.One.Member, as opposed to simply Has.Member or even Has.One.SameAs. It could be we are letting an unimplemented sequence through when it should not be allowed - or should be implemented if it is allowed.

@mikkelbu
Copy link
Member

mikkelbu commented Dec 4, 2017

@Snaaio As far as I can tell from the PR you can write the assertion as

Assert.That(new List<TestElement> { element }, Has.One.EqualTo(element));

But note that this will also be satisfied by a collection with more than one element, as long as exactly one element is equal to element. If you only want to match a collection with exactly one element that matches, then something like

Assert.That(new List<TestElement> { element, element2 }, Has.Member(element).And.Count.EqualTo(1));

can be used (note that I'm usually not using these long method sequences, so it can be that it can be written more succinct 😄 ).

@ChrisMaddock The Has.One syntax is new in 3.8 and is a short hand for Has.Exactly(1).Items (see #2004).

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

Successfully merging a pull request may close this issue.

6 participants