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

[attribute-behavior] Snapshot test #10587

Closed
wants to merge 2 commits into from

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Sep 1, 2017

Same data as the attribute table, but in the form of a snapshot. Uses puppeteer to run headless Chromium.

@acdlite
Copy link
Collaborator Author

acdlite commented Sep 1, 2017

If we're not ready to check in the actual snapshot file yet, because we aren't happy with the output, I would prefer to check in the rest of the changes in this PR (like extracting the config into a separate module) so I don't have to keep rebasing whenever we change the table.

@nhunzaker
Copy link
Contributor

Ah, this is super cool. I was wondering if we'd ever do headless testing of this stuff.

@gaearon
Copy link
Collaborator

gaearon commented Sep 1, 2017

@nhunzaker Do you want to help out reviewing this PR?

Same data as the attribute table, but in the form of a snapshot. Uses
puppeteer to run headless Chromium.
@acdlite
Copy link
Collaborator Author

acdlite commented Sep 1, 2017

Rebased

return;
}

if (!didStartTest && str.includes('http://localhost:9292/')) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's probably a better way to do this but idk what it is :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What I really want is a callback that says "Hey CRA is serving your app now, go ahead and run your tests."

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd vote do a production build and run at as a static for serve, if possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • that way you have direct control of setup and teardown without any ipc

@gaearon
Copy link
Collaborator

gaearon commented Sep 1, 2017

How does this work? Does it build the bundles first?

@acdlite
Copy link
Collaborator Author

acdlite commented Sep 1, 2017

Nah, just runs CRA dev server. But you're right, would probably be easier to just build the bundle and then serve a static file. I think maybe puppeteer lets you give it a file.

@gaearon
Copy link
Collaborator

gaearon commented Sep 1, 2017

The thing that bothers me is that this doesn't solve the warning deduplication problem (afaik). I always edit the bundle to remove warnedProperties[ and warnedProperties$1[ assignments because they make the warning reporting look wrong. But it seems like snapshot test would fall to this problem.

@acdlite
Copy link
Collaborator Author

acdlite commented Sep 1, 2017

I see. Could we add a secret feature flag?

@gaearon
Copy link
Collaborator

gaearon commented Sep 1, 2017

¯\(ツ)

@gaearon
Copy link
Collaborator

gaearon commented Sep 6, 2017

Superseded by #10595.
Do you think we should still make a script that presses "Save" via puppeteer?

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.

4 participants