1

I have looked at a few posts such as this one but it doesn't answer my question.

Basically I have use LINQKIT to build an expression which looks like this:

public Expression<Func<uv_Manifest, bool>> CreateManifestFilters(ManifestFilterItem filterItem)
{
    var predicate = PredicateBuilder.New<uv_Manifest>;

        if(!string.IsNullorWhiteSpace(filterItem.CID)){
        predicate = predicate.And(x => x.CID == filterItem.CID && x.CID != null);
        }

    return predicate;
}

And I have a unit tests which looks like this:

[TestMethod()]
public void CreateManifestFilters_FunctionHitWithCIDPopulated_ExpressionWillContainWhereOnCID()
{
    var filterItem = new ManifestFilterItem("002");
    var predicate = PredicateBuilder.New<uv_Manifest>.And(x => x.CID == filterItem.CID && x.CID != null);

    var result = _iManifestManager.CreateManifestFilters(filterItem);

    var manifest = new uv_Manifest();

    Assert.AreEqual(predicate, result);

}

What I am trying to do is check that the expression is doing a where on the property CID. However I get this error when the test fails:

Assert.AreEqual failed. Expected: ((CompareString(x.CID, value(FSVendor_Refactored.Tests.ManifestFixture+_Closure$__11-0).$VB$Local_filterItem.CID, False) == 0) AndAlso (Convert(x.CID) != null))>. Actual: ((CompareString(x.CID, value(FSVendor_RefactoredRepository.ManifestManager+_Closure$__2-0).$VB$Local_filterItem.CID, False) == 0) AndAlso (Convert(x.CID) != null))>.

I think I understand why it's failing, both the unit tests and CreateManifestFilters function are in different projects. So I believe that this is causing the test to fail.

Anyone know how to check the expression to check if it's doing a where on the CID property?

EDIT: This is NOT a duplicate as the other answer does not use PredicateBuilder therefore the linked answer doesn't suit my requirements.

Erik Philips
  • 53,428
  • 11
  • 128
  • 150
Andrew
  • 720
  • 3
  • 9
  • 34
  • 2
    It defeats the purpose of a unit test when the unit test is just executing the exact same code as the method it's testing. Even if you fixed it to properly compare the results, it's of no value to have a unit test that's just exactly the same as the method it's testing. – Servy Nov 13 '17 at 20:35
  • Hey, I'm not trying to unit test the results of the expression. I do have some code in my predicate builder which checks if CID is null or empty and if it is, it will skip the expression on CID. Essentially im trying to test that if an IF returns true, then the expression will be created which considers the CID – Andrew Nov 13 '17 at 20:37
  • 2
    Probably the easiest way is just to compile the expression and unit-test the compiled function. – StriplingWarrior Nov 13 '17 at 21:12
  • 1
    Wait, so you think that having a solution for how to compare any two arbitrary expressions, regardless of how you got them, doesn't solve your problem because you happened to use a particular tool to construct those expressions? How you created the expressions is irrelevant to how you go about comparing if they're identical. – Servy Nov 13 '17 at 21:17
  • @Servy, you are correct. I apologise. I was in a rush to go to my lunch and didn't modify the code to suit my requirements. – Andrew Nov 13 '17 at 22:03

1 Answers1

1

What I am trying to do is check that the expression is doing a where on the property CID.
...
Anyone know how to check the expression to check if it's doing a where on the CID property?

Yes, I can think of a few ways. The easiest is probably to compile the expression and test it.

[TestMethod()]
public void CreateManifestFilters_FunctionHitWithCIDPopulated_ExpressionFiltersOnCID()
{
    var filterItem = new ManifestFilterItem("002");
    var predicate = _iManifestManager.CreateManifestFilters(filterItem);
    var predicateFunc = predicate.Compile();
    var manifest1 = new uv_Manifest{ CID = "001" };
    var manifest2 = new uv_Manifest{ CID = "002" };

    var result1 = predicateFunc(manifest1);
    var result2 = predicateFunc(manifest2);

    Assert.IsFalse(result1);
    Assert.IsTrue(result2);
}

Hey, I'm not trying to unit test the results of the expression.

Why not? You specifically said you wanted to check that the expression is doing a WHERE on the CID property. Is the behavior of the expression not as important as knowing you hit the if block? In that case, it may be more appropriate to have another injected service providing the CID-checking expression so you can unit-test this method by mocking that service. But that seems like overkill to me.

Another option, if you just want to make sure that something CID-related is on that expression, is to ToString() the expression and look for a specific substring, like "filterItem.CID" in that string.

This is NOT a duplicate as the other answer does not use PredicateBuilder therefore the linked answer doesn't suit my requirements.

Actually, if you're really trying to just do an equality comparison between two expression trees, as you indicate in your code sample, then it doesn't matter how you generate the expressions. If they're equivalent, then the other answer's expression checker should work just fine.

StriplingWarrior
  • 151,543
  • 27
  • 246
  • 315
  • Trying to test a compiled expression isn't going to do a good job of replicating the actual requirements, as there are so many operations that support being compiled and run in code that won't properly translate into SQL. There's also the reverse problem; things that won't work if compiled and run, but that will work just fine when passed to a query provider. – Servy Nov 13 '17 at 21:51
  • @Servy: Yes, that's a technicality that could bite people if they're entirely relying on this method of testing to know that their code is correct. Hopefully they'll be doing some integration tests to ensure that the queries actually run. But I see it as outside the scope of this question. – StriplingWarrior Nov 13 '17 at 21:56
  • If you're not going to actually use the test to test the code, then why have the test in the first place? It's not helping anything. If you're going to actually take the time to write test, you should ensure that the tests actually test that the code works, rather than writing tests filled with false positives and false negatives. Having no (automated) tests is better than tests with lots of incorrect results. – Servy Nov 13 '17 at 21:57
  • @Servy: Choosing what pieces of code to test, how many tests to write, and which kind they should be, etc., are decisions that we all need to make every day based on our knowledge of the code base, project priorities, lifecycle, team size, and a variety of other factors not captured in a SO question. If the OP feels that it's important to test that this particular method returns a particular type of expression, I may offer alternative suggestions, but I won't judge. I've seen tests like the one I wrote here provide value in the past, without yielding false positive or false negatives. – StriplingWarrior Nov 14 '17 at 06:28
  • Yes, choosing what code to test, how many test to write, and how to test code, are indeed decisions that you need to make. The question is not whether or not *to* test it, or how much code coverage to have, it's about having *correct* tests. You've constructed a test that reliably produces incorrect results. That's more harmful than not having a test. If the OP decides they want to test that code, that's fine, but they need to use *a valid test* that actually correctly identifies whether or not the method is working. This doesn't do that. – Servy Nov 14 '17 at 14:05
  • @Servy What approach would you suggest in this situation? I'm assuming it's one that I haven't mentioned? – StriplingWarrior Nov 14 '17 at 16:10
  • Well there's the actual *working* solution that you removed from the question for no valid reason whatsoever. – Servy Nov 14 '17 at 16:16
  • @Servy: Are you talking about the link that got removed automatically when I reopened the question? The one that I linked to in my answer? I had the impression you were against using that approach based on your comment: "Even if you fixed it to properly compare the results, it's of no value to have a unit test that's just exactly the same as the method it's testing." I feel like I'm missing something important. Can you clarify? Maybe add an answer of your own? – StriplingWarrior Nov 14 '17 at 17:11
  • Why would I post an answer to a duplicate question just to repeat what the duplicate question's answers said? When a question is a duplicate, you close it as a duplicate, you don't abusively reopen the question, knowing that it's a duplicate, just to repeat the same information in the duplicate. – Servy Nov 14 '17 at 17:52
  • @Servy: When I reopened the question, I actually thought that it wasn't a duplicate. Like the OP, I apologize. I've re-closed it now. I'm still confused about what you meant by your original comment on the question, since you seem to be saying that following the duplicate question won't actually produce a decent unit test. I would like to learn from you, but your tone lately has been really antagonistic and you seem more bent on telling everybody why their ideas are stupid than trying to help them learn. I hope things are okay for you. – StriplingWarrior Nov 14 '17 at 18:11
  • My first comment on the question referred to the fact that, in the original revision, the unit test code and the method's code were literally textually identical. They've since edited the question to add complexity to the actual method not in the unit test, making it a meaningful unit test to have (it could be better, but it's no longer pointless). – Servy Nov 14 '17 at 18:18
  • I apologize if you're off put that I've been frustrated that someone would remove a valid and working solution to a problem in order to replace it with something that doesn't work at all and causes more problems than it solves, and that they wouldn't care at all about having done something like that. To me that's a serious problem. If it means I'm not okay to care about a question having a working answer, then I don't want to be "okay". – Servy Nov 14 '17 at 18:18
  • That makes more sense. Thank you for explaining. – StriplingWarrior Nov 14 '17 at 19:23