Skip to content

chore(live-preview): load schemaJSON from proper client config in integration tests #12167

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

Conversation

andershermansen
Copy link
Contributor

@andershermansen andershermansen commented Apr 20, 2025

What?

The integration tests in live-preview has been using the fieldSchemaToJSON method with wrong params/types.

It's defined as

export const fieldSchemaToJSON = (fields: ClientField[], config: ClientConfig): FieldSchemaJSON

In the test setup
fields was set to Pages.fields which was Field[], not the expected ClientField[]
config was set to config which was Promise<SanitizedConfig> not the expected ClientConfig

Why?

I'm working on some other changes to live-preview where I need the proper values wired up correctly to properly add integration tests.

The test has worked up until now because Field is very similar to ClientField. But it should test with the correct type.

How?

By creating the clientConfig and using the correct types/params when calling fieldSchemaToJSON in the test setup.

Note: Removed test "Collections - Live Preview › merges data", the test worked before because id field is not part of Field, but part of ClientField. So test code does not behave like this in real scenario when real ClientField is used. There are lots of real tests for correct data, removed this one which seems very simple and not correct.

@andershermansen
Copy link
Contributor Author

  ● Collections - Live Preview › merges data

    expect(received).toEqual(expected) // deep equality

    Expected: "123"
    Received: undefined

      169 |     })
      170 |
    > 171 |     expect(mergedData.id).toEqual(initialData.id)
          |                           ^
      172 |     expect(mergedData._numberOfRequests).toEqual(0)
      173 |   })
      174 |

      at Object.toEqual (live-preview/int.spec.ts:171:27)

This test is failing.

If I understand the code correctly this is because Field does not contain id field but ClientField does. So the mergeData will traverse the fields and after the change to ClientField also traverse the id field and set it with undefined here

I'm not really sure what is expected here, if the test or the implementation is wrong. So need some feedback.

Copy link
Contributor

@GermanJablo GermanJablo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I also understand that the test is incorrect, and that there are many others that test the same function with correct data. LGTM!

@GermanJablo GermanJablo changed the title chore(live-preview): load schemaJSON from proper client config chore(live-preview): load schemaJSON from proper client config in integration tests May 22, 2025
@GermanJablo GermanJablo merged commit 08a3dfb into payloadcms:main May 22, 2025
78 checks passed
Copy link
Contributor

🚀 This is included in version v3.39.0

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

Successfully merging this pull request may close these issues.

2 participants