Page MenuHomePhabricator

Fix dagit tests, switch to smarter styled-components autogen classnames

Authored by bengotow on Dec 14 2019, 5:16 PM.



At first I just wanted to upgrade our usage of styled components to the new "compile-time macro" version that creates reasonable classnames that are "stable" and include the React component name.

However, I noticed that after I did this it did not break the tests which was very unusual.

It turns out our tests have been broken for a while - the snapshots are matching, but the snapshots contain pages showing the "GraphQL Error" bar and no content.

I fixed this and made several changes to prevent this sort of thing from happening again:

  • The mock queries use the TypeScript types for the query variables, ensuring that if variables are changed we realize we need to update these queries.
  • The simple "load the explorer" test checks against the snapshot but also verifies that the graph component is present as a sanity check (to make sure the snapshot doesn't just contain a "GraphQL Error" toast.
  • The script that re-generates the mock query data now checks the responses and throws an exception if dagit returns an error for a mocked query.
  • All of the miscellaneous react-test-renderer warnings have been resolved, so the Dagit tests should run with no red output. Previously there were some red herrings related to react hooks that were making it difficult to see real problems, like the fact that queries were going unmatched against the test set.
Test Plan

Run tests that actually work now

Diff Detail

R1 dagster
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bengotow created this revision.Dec 14 2019, 5:16 PM
bengotow edited the summary of this revision. (Show Details)Dec 14 2019, 5:17 PM
This revision is now accepted and ready to land.Dec 14 2019, 5:38 PM