Port Relay test tooling from Reaction, update to Node 10 #1209

javamonn · 2018-10-30T14:48:47Z

Ports Relay DevTools from reaction: renderUntil, renderRelayTree, MockRelayRenderer. As part of this work, made Enzyme static rendering work via jsdom. This is a stop gap (RN is not the DOM), and we'll want to track Create Adapter for React Native & React 16 enzymejs/enzyme#1436 for first class RN static rendering support in enzyme.
Updates Node from 8.4 to 10.13.0, the latest LTS.

#skip_new_tests

I ported the existing tests in Reaction over, but not all new files had/need test coverage.

peril-staging · 2018-10-30T14:49:25Z

New dependencies added: relay-mock-network-layer.

relay-mock-network-layer

Author: Rob Richard

Description: Relay modern network layer that returns schema correct mock data

Homepage: https://github.com/1stdibs/relay-mock-network-layer#readme


Created	about 1 year ago
Last Updated	7 days ago
License	MIT
Maintainers	2
Releases	6
Direct Dependencies	`graphql-tools`

README

relay-mock-network-layer

Provides a network layer for relay modern that returns schema-correct mock data using addMockFunctionsToSchema from graphql-tools;

This is useful for working in an environment like storybook where you want to work on your components without hitting a live GraphQL server.

Usage

import {
	Environment,
	Network,
	RecordSource,
	Store
} from 'relay-runtime';
import getNetworkLayer from 'relay-mock-network-layer';
import schema from './graphql.schema.json';

const network = Network.create(getNetworkLayer({
    schema,
    // pass custom mocks as documented in graphql-tools
    // http://dev.apollodata.com/tools/graphql-tools/mocking.html#Customizing-mocks
    mocks: {
        Geography: () => ({
            id: '2',
            countries: [{
                abbreviation: 'US',
                name: 'United States'
            }, {
                abbreviation: 'UK',
                name: 'United Kingdom'
            }],
                usStates: [{
                abbreviation: 'NY',
                name: 'New York'
            }, {
                abbreviation: 'NJ',
                name: 'New Jersey'
            }]
        }),
        Address: () => ({
            country: 'US',
            city: 'New York',
            state: 'NY',
            zipCode: '10012',
        })
    }
}));

// Create an environment using this network:
const store = new Store(new RecordSource());
const environment = new Environment({network, store});

// use environment in <QueryRenderer>

Mocking custom scalar types

If your schema has custom scalar types you'll need to use the resolvers option to ensure those types get mocked correctly. Pass this option an object containing a resolve function for each custom scalar.

...
import getNetworkLayer from 'relay-mock-network-layer';
import {GraphQLScalarType} from 'graphql';

...

getNetworkLayer({
    schema,
    mocks: {...},
    resolvers: {
        CustomScalar: new GraphQLScalarType({
            name: 'CustomScalar',
            parseLiteral: () => {},
            parseValue: () => {},
            serialize: () => {}
        }),
        CustomScalar2: ...
    }
});

Generated by 🚫 dangerJS

DangerCI · 2018-10-30T14:51:22Z

	Warnings
⚠️	This PR comes from a fork, and won't get JS generated for QA testing this PR inside the Emission Example app.
⚠️	Missing Test Files: `src/lib/Components/__tests__/Spinner-tests.tsx` `src/lib/tests/__tests__/MockRelayRenderer-tests.tsx` `src/lib/tests/__tests__/RelayStubProvider-tests.ts` `src/lib/tests/createMockNetworkLayer/CustomScalars/__tests__/formatted_number-tests.ts` `src/lib/tests/createMockNetworkLayer/__tests__/index-tests.ts` `src/lib/utils/__tests__/Context-tests.tsx` If these files are supposed to not exist, please update your PR body to include "#skip_new_tests".

Generated by 🚫 dangerJS

javamonn · 2018-10-30T14:53:03Z

package.json

@@ -30,7 +30,7 @@
    "test": "jest",
    "test:ci": "jest --outputFile test-results.json --json --runInBand",
    "testing": "jest --watch",
-    "relay": "relay-compiler --src ./src --schema data/schema.graphql --language typescript --artifactDirectory ./src/__generated__ --persist --persistOutput ./src/__generated__/complete.queryMap.json",
+    "relay": "relay-compiler --src ./src --schema data/schema.graphql --language typescript --artifactDirectory ./src/__generated__ --exclude '**/node_modules/**,**/__mocks__/**,**/__generated__/**'",


relay-mock-network-layer, used in createMockNetworkLayer, depends on query text being present in the operation in order to client side resolve. Our current approach of used persisted queries however sets text to null in generated .graphql artifacts so that queries are not sent over the network. We obviously don't want to disable persisted queries just to get these tests to work.

Some ideas @orta had to workaround this:

Update compiler to include query text as some other field in the generated artifact, presumably wouldn't be sent over the wire at runtime as the runtime wouldn't know about the generated field.

Update relay-mock-network-layer to resolve operation text differently

javamonn · 2018-10-30T15:07:52Z

src/lib/tests/renderRelayTree.tsx

+ * tree.
+ */
+export const RelayFinishedLoading: RenderUntilPredicate<any, any, any> = tree =>
+  !tree.find(`[testID=${LoadingTestID}]`).length


The implementation of renderRelayTree in reaction uses a class name as a loading sentinel. RN does not have classes, and (I think?) we need something that is flushed to a view primitive. I opted for View's testID prop. It comes with a caveat:

This disables the 'layout-only view removal' optimization for this view!

~~Though I think this is safe to ignore as we only pass it during tests.~~ This gets passed wherever renderWithLoadProgress is used, though I still believe the affected surface area is small - this would only affect Spinner.

orta · 2018-10-30T15:13:56Z

src/__generated__/ActiveBidsQuery.graphql.ts

-  "id": "ff999f5e1d50496e9b0dcfc8e6561436",
-  "text": null,
+  "id": null,
+  "text": "query ActiveBidsQuery {\n  me {\n    ...ActiveBids_me\n    __id\n  }\n}\n\nfragment ActiveBids_me on Me {\n  lot_standings(live: true) {\n    most_recent_bid {\n      __id\n    }\n    ...ActiveBid_bid\n  }\n  __id\n}\n\nfragment ActiveBid_bid on LotStanding {\n  is_leading_bidder\n  sale {\n    href\n    is_live_open\n    __id\n  }\n  most_recent_bid {\n    __id\n    max_bid {\n      display\n    }\n    sale_artwork {\n      artwork {\n        href\n        image {\n          url\n        }\n        artist_names\n        __id\n      }\n      counts {\n        bidder_positions\n      }\n      highest_bid {\n        display\n        __id: id\n      }\n      lot_number\n      reserve_status\n      __id\n    }\n  }\n}\n",


This ^ is the query text needed in the mock layer ^

javamonn · 2018-10-30T15:14:14Z

src/setupJest.ts

+}
+
+global.window = new JSDOM("<!doctype html><html><body></body></html>").window
+global.document = window.document


This was copied over from the approach seen here: enzymejs/enzyme#1436 (comment)

This is preferable (imo) to the alternate approach seen further up that thread (enzymejs/enzyme#1436 (comment)) and posted in slack of react-native-mock-render as it requires no component mocking. Still, this approach is non-ideal as RN is not the DOM, but this is enough to get Enzyme to work for the time being. Ideally Enzyme would support RN first-class with an RN adapter, which enzymejs/enzyme#1436 tracks discussion of.

javamonn · 2018-10-31T19:43:55Z

package.json

    "graphql": "^0.13",
    "relay-runtime": "https://github.com/alloy/relay/releases/download/v1.5.0-artsy.5/relay-runtime-1.5.0-artsy.5.tgz",
-    "@types/relay-runtime": "^1.3.5"
+    "@types/relay-runtime": "^1.3.5",
+    "cheerio": "0.22.0"


jest-environment-jsdom depends on cheerio@^1.0.0-rc.2. I ran into a serialization error with this version, where <Image /> elements rendered to string would be transformed into <img> instead of the expected <image />:

renderRelayTree › resolves a promise once the full tree (including nested query renderers) has been rendered expect(received).toEqual(expected) Expected value to equal: "<view><img source=\"[object Object]\"><text ellipsizemode=\"tail\">Mona Lisa</text><text ellipsizemode=\"tail\">Leonardo da Vinci</text></view>" Received: "<view><image source=\"[object Object]\"></image><text ellipsizemode=\"tail\">Mona Lisa</text><text ellipsizemode=\"tail\">Leonardo da Vinci</text></view>"

My hunch is that enzyme must be passing slightly different configuration options down to cheerio when calling render vs mount, but not sure. Resolving to the latest non-rc cheerio version fixes this, but does appear to be a hack.

javamonn · 2018-10-31T19:45:27Z

package.json

@@ -61,10 +61,10 @@
    "lib"
  ],
  "resolutions": {
-    "parse5": "5.1.0",


This resolution does not appear to be necessary anymore. Original discussion here: https://github.com/artsy/emission/pull/1185/files#r217160876

javamonn · 2018-10-31T19:46:53Z

package.json

@@ -116,12 +116,13 @@
    "enzyme": "^3.1.0",
    "enzyme-adapter-react-16": "^1.0.2",
    "husky": "^0.14.3",
-    "jest": "^22.4.2",
+    "jest": "^23.6.0",


I had to upgrade jest as part of upgrading to node 10 due to needing this PR: jestjs/jest#6505

javamonn · 2018-10-31T19:58:39Z

patches/relay-mock-network-layer+1.3.0.patch

+-            // See https://github.com/facebook/relay/issues/1816
+    const query =
+      operation.text ||
+      (resolveQueryFromOperation && resolveQueryFromOperation(operation));


Lots of diff in this file due to running Prettier against it, but this is the only meaningful change. @orta and I paired on adding this in in response to the issue described in the frontend practice meeting. To put it into text: client side query resolving requires access to the relay query operation text which is not immediately available due to Emission's use of persisted queries, where only the ID is in the compiled artifact. This change allows us to pass in a query resolver function, where we then use the same approach used in fetchQuery.ts to resolve the ID in the query map.

Our usage of this can be seen here: https://github.com/javamonn/emission/blob/084f721f766476e497ecd078add48c5de5050db8/src/lib/tests/createMockNetworkLayer/index.ts#L21-L23

I will PR these changes against the upstream repository once we land them here.

Heads up that I have a PR over in relay-mock-network-layer that will allow us to pass additional options over: 1stdibs/relay-mock-network-layer#7

@javamonn @orta Can you elaborate on why you decided against disabling persisted queries in development?

Mainly that it increases the difference between dev and prod

javamonn · 2018-10-31T20:00:13Z

src/lib/Components/Artist/Articles/__tests__/__snapshots__/Article-tests.tsx.snap

@@ -9,13 +9,7 @@ exports[`renders properly 1`] = `
  }
 >
  <View
-    accessibilityComponentType={undefined}


I had to regenerate all snapshots, I'm assuming due to having upgraded Jest.

javamonn · 2018-10-31T20:05:21Z

There's a fair amount of console.error noise in the jest output which is expected due to the jsdom setup, but I'll look into if there's a way we can somehow selectively silence these:

    console.error node_modules/fbjs/lib/warning.js:33
      Warning: React does not recognize the `ellipsizeMode` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `ellipsizemode` instead. If you accidentally passed it from a parent component, remove it from the DOM element.
          in Text (created by Text)
          in Text (created by Component)
          in View (created by View)
          in View (created by Component)
          in Component (created by WrapperComponent)
          in WrapperComponent
    console.error node_modules/fbjs/lib/warning.js:33
      Warning: <View /> is using uppercase HTML. Always use lowercase HTML tags in React.
    console.error node_modules/fbjs/lib/warning.js:33
      Warning: The tag <View> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.
    console.error node_modules/fbjs/lib/warning.js:33
      Warning: <Text /> is using uppercase HTML. Always use lowercase HTML tags in React.
    console.error node_modules/fbjs/lib/warning.js:33
      Warning: <View /> is using uppercase HTML. Always use lowercase HTML tags in React.

javamonn · 2018-10-31T20:09:03Z

src/lib/tests/createMockNetworkLayer/index.ts

+// Typescript thinks we're in React Native, this file should be test-only i.e. executed within Node.
+declare const __dirname: string
+// @ts-ignore
+import * as fs from "fs"


Is there a better way to make TS happy here? ts-jest takes a tsConfig option that looks promising, but I'm not knowledgeable enough about how the current setup works.

Yeah, but that's worth doing in a separate PR IMO

damassi · 2018-10-31T20:22:15Z

src/lib/tests/Context.tsx

+  return <Context.Provider value={providerValues}>{children}</Context.Provider>
+}
+
+export const ContextConsumer = Context.Consumer


Should this context provider be moved out of the tests folder?

Yup, nice call. Stuck it in there as only MockRelayRenderer depends on it, but no reason we can't start using it more widely.

damassi · 2018-10-31T20:25:25Z

Very nice PR @javamonn 💯

orta · 2018-11-01T14:11:49Z

Yep, cool - will rebase for you

javamonn commented Oct 30, 2018

View reviewed changes

orta reviewed Oct 30, 2018

View reviewed changes

javamonn commented Oct 30, 2018

View reviewed changes

javamonn force-pushed the port-relay-devtools branch 3 times, most recently from 58b7ac1 to d4d2df8 Compare October 31, 2018 19:13

javamonn commented Oct 31, 2018

View reviewed changes

javamonn changed the title ~~WIP: Port Relay test tooling from Reaction~~ Port Relay test tooling from Reaction, update to Node 10 Oct 31, 2018

javamonn assigned orta Oct 31, 2018

javamonn requested review from damassi, alloy and l2succes October 31, 2018 20:03

javamonn commented Oct 31, 2018

View reviewed changes

damassi reviewed Oct 31, 2018

View reviewed changes

javamonn added 8 commits October 31, 2018 16:38

Port Relay DevTools from Reaction

d695b45

Patch relay-mock-network-layer to support persisted query resolution

133068b

Port over MockRelayRendererFixtures, renderWithLoadProgress tests

89c5202

Update to Node 10 and update affected testing dependencies

a765905

Remove unused RAF dep

ab692ae

Add Changelog entry

6df6bf4

Lift Context out of tests and into utils

eec0d25

Post-rebase lockfile updates

31aab9a

javamonn force-pushed the port-relay-devtools branch from bbe161a to 31aab9a Compare October 31, 2018 20:41

Merge branch 'master' into port-relay-devtools

72f7553

javamonn mentioned this pull request Nov 1, 2018

Add resolveQueryFromOperation and JSDoc comments 1stdibs/relay-mock-network-layer#8

Merged

orta merged commit 7ed93f4 into artsy:master Nov 1, 2018

javamonn mentioned this pull request Nov 7, 2018

Cleanup Jest output #1220

Merged

javamonn mentioned this pull request Nov 29, 2018

State of testing Relay powered components artsy/reaction#1604

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port Relay test tooling from Reaction, update to Node 10 #1209

Port Relay test tooling from Reaction, update to Node 10 #1209

javamonn commented Oct 30, 2018 •

edited

Loading

peril-staging bot commented Oct 30, 2018 •

edited

Loading

relay-mock-network-layer

Usage

Mocking custom scalar types

DangerCI commented Oct 30, 2018 •

edited

Loading

javamonn Oct 30, 2018

javamonn Oct 30, 2018

javamonn Oct 30, 2018 •

edited

Loading

orta Oct 30, 2018

javamonn Oct 30, 2018

javamonn Oct 31, 2018

javamonn Oct 31, 2018

javamonn Oct 31, 2018

javamonn Oct 31, 2018 •

edited

Loading

damassi Oct 31, 2018

alloy Nov 1, 2018

orta Nov 1, 2018

alloy Nov 1, 2018

javamonn Oct 31, 2018

javamonn commented Oct 31, 2018

javamonn Oct 31, 2018

orta Nov 1, 2018

damassi Oct 31, 2018

javamonn Oct 31, 2018

damassi commented Oct 31, 2018

orta commented Nov 1, 2018 •

edited

Loading

Port Relay test tooling from Reaction, update to Node 10 #1209

Port Relay test tooling from Reaction, update to Node 10 #1209

Conversation

javamonn commented Oct 30, 2018 • edited Loading

peril-staging bot commented Oct 30, 2018 • edited Loading

relay-mock-network-layer

Usage

Mocking custom scalar types

DangerCI commented Oct 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javamonn Oct 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javamonn Oct 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javamonn commented Oct 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damassi commented Oct 31, 2018

orta commented Nov 1, 2018 • edited Loading

javamonn commented Oct 30, 2018 •

edited

Loading

peril-staging bot commented Oct 30, 2018 •

edited

Loading

DangerCI commented Oct 30, 2018 •

edited

Loading

javamonn Oct 30, 2018 •

edited

Loading

javamonn Oct 31, 2018 •

edited

Loading

orta commented Nov 1, 2018 •

edited

Loading