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

t.like not handling arrays as expected #2627

Closed
awlayton opened this issue Dec 19, 2020 · 16 comments · Fixed by #3185
Closed

t.like not handling arrays as expected #2627

awlayton opened this issue Dec 19, 2020 · 16 comments · Fixed by #3185

Comments

@awlayton
Copy link

I am trying to use the t.like assertion on deep objects with arrays of objects in them.

The following simplified example fails:

import test from 'ava';

test('deep t.like', (t) => {
  t.like({ a: [{ a: 1, b: 2 }] }, { a: [{ a: 1 }] });
});
> tsc -b

$ ava dist/ava.spec.js

  deep t.like

  Difference:

    {
      a: [
        {
          a: 1,
  -       b: 2,
        },
      ],
    }

  › src/ava.spec.ts:4:5

  ─

  1 test failed
error Command failed with exit code 1.

I expected the above assertion to pass.
It seems that elements within arrays need to be deep equal rather than "like" for the assertion to pass.

AVA version:

$ ava --version
3.14.0
@ninevra
Copy link
Contributor

ninevra commented Dec 19, 2020

(Not a maintainer, but)

This is behaving as documented: "Any values in selector that are not regular objects should be deeply equal to the corresponding values in value." What a "regular object" is in javascript is admittedly not well defined, but arrays aren't it.

It would sometimes be useful for t.like() to recurse into arrays, but then there would be lots of design questions to answer: Should order matter? Should extra trailing elements in the sample matter? Should Maps and Sets also be recursed into? Adding such special-cases to t.like() would make it less clear what any given assertion is actually checking.

One can achieve the behavior you expect either by writing one's own selector function and then using t.deepEqual(), or by using an outside assertion library.

@awlayton
Copy link
Author

I had interpreted it as anything that was JSON serializable. But I could see them meaning it more literally.

What do you mean about t.deepEqual? Is there some undocumented way to use it? It just takes 2 values to compare and an optional message.

@ninevra
Copy link
Contributor

ninevra commented Dec 19, 2020

Internally, what t.like(actual, selector) is doing is constructing a subset of actual and then comparing it by deep equality to selector; i.e. there exists some function select(actual, selector) -> sample such that t.like(actual, selector) is equivalent to t.deepEqual(select(actual, selector), selector).

(The selector function used by AVA is in lib/like-selector.js)

So if you want to recurse into arrays, you could write a selector function something like:

function selectWithArrays(actual, selector) {
  if (isPlainObject(selector)) {
    // ...
  } else if (Array.isArray(selector) && Array.isArray(actual)) {
    const selected = [];
    for (const [index, value] of selector.entries()) {
      selected.push(selectWithArrays(actual[index], value));
    }
    return selected;
  } else {
    return actual;
  }
}

and then use t.deepEqual(selectWithArrays(actual, selector), selector);

@novemberborn
Copy link
Member

@ninevra is correct.

like uses deepEqual under the hood, which, like snapshot uses https://github.com/concordancejs/concordance to determine equality.

Ultimately a "partial equal" feature could be added there, allowing us to be smarter with say (primitive-value) sets and maps, but even then deciding how two arrays or complex-value sets are partially equal becomes difficult.

(I'm closing this issue for housekeeping purposes, but let's keep the conversation going.)

@awlayton
Copy link
Author

awlayton commented Jan 4, 2021

I was expecting it to just treat an array the way it treats and object. That is, element i of the array (key i of it as an object) must be "like" element i of the corresponding selector array.

Not saying that is the only interpretation for arrays, but it is one way.

@novemberborn
Copy link
Member

Yea, I appreciate that. I'm not quite sure how we'd build that on top of Concordance though, especially with nested arrays.

@awlayton
Copy link
Author

awlayton commented Jan 5, 2021

I have no experience with concordance, but could you not simply transform any arrays to their "equivalent" object before doing the comparison with the selector?

@novemberborn
Copy link
Member

You'd have to walk the object tree to find all those arrays. It's fine for simple cases but much harder for all cases.

@awlayton
Copy link
Author

awlayton commented Jan 6, 2021

Not sure I see cases where it is hard, unless you are worried about it becoming slow for giant objects? You just need to recurse down the thing.

@novemberborn
Copy link
Member

Recursing is the problem. For example it has to work for sets and maps (keys included).

@fungiboletus
Copy link

I'm migrating tests from Jest to Ava and we use .toMatchObject that has an acceptable behaviour, it's like like in Ava, but it manages arrays. The order simply needs to be the same.

@Gozala
Copy link

Gozala commented May 12, 2022

I have run into the same problem, and also find handling of arrays surprising. I have identified problem to be caused by

&& Reflect.getPrototypeOf(selector) === Object.prototype

Which I was able to address locally with small changes
#3023

@novemberborn
Copy link
Member

The solution proposed by @Gozala in #3023 may prove worthwhile, though I haven't thought it through in much detail. Reopening for further exploration.

@novemberborn novemberborn reopened this Jun 6, 2022
@Greegko
Copy link

Greegko commented Jan 8, 2023

FYI, I applied @Gozala changes and patching the lib with it, so far it works for me without any issue.

@novemberborn
Copy link
Member

@Greegko would you like to pick up #3023 and turn it into a fresh PR?

@Greegko
Copy link

Greegko commented Jan 25, 2023

@novemberborn yeah I will try to create tests around it 👍

tommy-mitchell pushed a commit to tommy-mitchell/ava that referenced this issue Mar 13, 2023
tommy-mitchell added a commit to tommy-mitchell/ava that referenced this issue Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants