Add fixture components #8860

jquense · 2017-01-24T20:49:32Z

related to: #8583

Let me know if this is way off base with what ya’ll are thinking :P Always fun to see how others build stuff

jquense · 2017-01-24T20:51:40Z

aweary · 2017-01-25T00:28:42Z

fixtures/dom/src/components/TestCase.js

+    <ol>
+      {children}
+    </ol>
+  </div>


Right now the fixtures can be tested with (almost) every version of React published. If we use functional components then it will break with versions <14.

It's probably safest to just use class components everywhere, but we can discuss limiting which versions we really need to support (probably not many)

Ya. I went with createClass for backward compatibility reasons, but that might cause forward compatibility issues I'm the future.

How far back do we want to support?

I didn't think about function components. but the farthest back version in the select is 0.13 which should be fine for plain class components

I think we're fine with only offering fixture tests for 0.13+, so we should probably convert all fixture components to use ES6 classes.

jquense · 2017-01-25T00:43:53Z

speaking of versions, does it make sense to add something like "fixed in version:" and known exceptions to the test case components. We probably want to document some sort of minimum version the test case should work in, to avoid regressions, or false positives for a regression

aweary · 2017-01-25T00:51:52Z

fixtures/dom/src/components/TestCase.js

+      <section className="test-case">
+        <h2 className="type-subheading">
+          <label>
+            <input type='checkbox' />


Maybe we can give the fixture test case a slight but noticeable green border when its checked off? That way it's very easy to scroll through quickly and see which tests you haven't done.

aweary · 2017-01-25T03:18:35Z

speaking of versions, does it make sense to add something like "fixed in version:" and known exceptions to the test case components.

Yes, it makes a lot of sense. We should also link to the PR that fixed it, so that if it regresses you can easily see how it was fixed the first time, which might give important context.

We probably want to document some sort of minimum version the test case should work in, to avoid regressions or false positives for a regression

👍 we don't backport bugfixes so this will be a must. Maybe we could automatically detect what version is being used and then only load those fixtures we expect to work in that version.

So, say, if an issue was resolved in 15.3 then it would specify that and would not be rendered if the loaded version was 15.2 or lower.

jquense · 2017-01-25T04:02:11Z

sounds good, I'll toss something together and add it in here 👍

nhunzaker · 2017-01-25T12:44:07Z

Keeping a manifest JS object of all the test cases, with a semver range might be a nice way to keep track of these. I could imagine it getting really useful as the count gets higher.

gaearon · 2017-01-25T13:19:41Z

FWIW I wouldn't worry too much about supporting 0.13 in fixtures.
But your call—whatever makes more sense.

nhunzaker · 2017-01-25T13:22:05Z

Is there an official stance on support for older versions of react?

gaearon · 2017-01-25T13:39:57Z

What do you mean?

nhunzaker · 2017-01-25T13:52:54Z

Like how far back bug fixes get migrated, and if, for example, if someone filed an issue for React 0.12, and it was fixed in 0.13 - would we backport the fix, or recommend they upgrade?

aweary · 2017-01-25T14:20:57Z

We don't really ever backport bug fixes, so we would likely be fine only having the current major version.

aweary · 2017-01-25T14:24:49Z

But as long as it doesn't introduce unneeded complexity I see no problem with allowing users to test with older major versions. So since 0.13 restricts us from using functional components, we might as well remove it from the list. We're not going to be backporting anything to it.

gaearon · 2017-01-25T14:38:15Z

We don’t backport fixes to previous majors apart from really bad cases at the very start of a new major release when we know people haven’t had time to migrate. Having just the previous and current major is enough.

jquense · 2017-01-25T15:20:47Z

I think since it's simple enough right now to support 0.13 in these test cases we might as well. it can be helpful to have a longer tail of past work to see how stuff behaved. even if we aren't going to fix anything for that version

jquense · 2017-01-25T20:44:03Z

clean up a bit and added some more options to TestCase. tried not to over engineer it. notable additions:

Flag test cases with a minimumVersion greatere than the selected one as "not expected to pass"
'affectedBrowsers' prop for optionally noting which browsers are known to be affected.

both are optional to allow usage of the component for cases that aren't tracking regressions or fixed bugs

aweary · 2017-01-25T20:47:22Z

fixtures/dom/src/components/Fixture.js

@@ -0,0 +1,33 @@
+import React from 'react';


For React and ReactDOM they should just be globally available. If we import it then we're only getting the current version that create-react-app installed. The react-loader script will load the selected version from a CDN.

oops that makes sense. force of habit

aweary · 2017-01-25T20:49:39Z

fixtures/dom/src/components/fixtures/input-change-events/index.js

+            Radio inputs should only fire change events when the checked
+            state changes.
+          `}
+          minimumVersion="16.0.0"


Maybe it would be helpful to also add resolvedIn and resolvedBy so we could give more details

<TestCase title="foo" resolvedIn="15.2.0" resolvedBy="#1760" />

Then it could say:

This test case was fixed in version 15.2.0 by #1760. This test is not expected to pass for the selected
version, and that's ok!

That would be really cool. How fancy a fixture tool we are making.

I'd be happy to pick up some of the work here. @jquense if you are busy on other things, I've got time to work on this bit. Would a PR to your branch be fine?

Or if you wanna jam on it, that's fine too. I just wanted to pick up some of the load.

go for it 👍 got caught up with work so feel free

nhunzaker · 2017-02-03T03:11:48Z

Sent out the PR feedback here: jquense#2

- Pull in React from window global - Add field to TestCase for fix PR links - Update some styles

aweary · 2017-02-13T23:37:16Z

@jquense @nhunzaker I rebased the PR with master and added a couple changes. The biggest once is a small utility that will cache the version tags for the duration of a session. Since the Github API is rate limited we want to minimize the number of requests we make.

Don't bust the cache doing feature detection COME ON

nhunzaker

Lookin good!

nhunzaker · 2017-02-13T23:41:38Z

fixtures/dom/src/components/FixtureSet.js

+    return (
+      <div>
+        <h1>{title}</h1>
+        {description  && (


Double spaces here?

nhunzaker · 2017-02-13T23:42:19Z

fixtures/dom/src/components/Header.js

      .then(tags => {
        let versions = tags.map(tag => tag.name.slice(1));
-        versions = ['local', ...versions];
+        versions = [`local (${React.version})`, ...versions];


Is this always going to be the current version you have loaded? So if you load 0.13.0, will it say local 0.13.0?

Yupp, I just removed it. It also caused problems when changing versions as it checks query.version === 'local'

nhunzaker · 2017-02-13T23:44:32Z

fixtures/dom/src/tags.js

+    if (canUseSessionStorage) {
+      cachedTags = sessionStorage.getItem(TAGS_CACHE_KEY);
+    }
+    console.log({ cachedTags })


Do we want to hold on to this log?

jquense · 2017-02-13T23:55:44Z

👍

aweary

Let's get this merged and start working on the fixture test cases so this doesn't get forgotten about.

facebook-github-bot added GH Review: review-needed CLA Signed labels Jan 24, 2017

aweary self-assigned this Jan 24, 2017

aweary reviewed Jan 25, 2017

View reviewed changes

jquense and others added 3 commits February 13, 2017 16:28

add fixture components

bc2fe08

add a few more options and styles

ed88f18

Test fixture updates

050bf3c

- Pull in React from window global - Add field to TestCase for fix PR links - Update some styles

aweary force-pushed the text-fixtures branch from 91370e3 to 050bf3c Compare February 13, 2017 22:29

Brandon Dail added 2 commits February 13, 2017 16:39

Remove unused Fixture.Result comment

a395436

Remove leading # from resolvedBy link

7e2676f

Implement tag loading utility that caches response

ad39395

Don't bust the cache doing feature detection COME ON

aweary force-pushed the text-fixtures branch from 8c20f9d to ad39395 Compare February 13, 2017 23:41

nhunzaker approved these changes Feb 13, 2017

View reviewed changes

Use 'local' without version for option

6e83dcc

sebmarkbage removed the GH Review: review-needed label Mar 1, 2017

aweary approved these changes Mar 2, 2017

View reviewed changes

aweary merged commit 2757a53 into facebook:master Mar 2, 2017

jquense deleted the text-fixtures branch March 2, 2017 20:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fixture components #8860

Add fixture components #8860

jquense commented Jan 24, 2017 •

edited

Loading

jquense commented Jan 24, 2017

aweary Jan 25, 2017

nhunzaker Jan 25, 2017

jquense Jan 25, 2017

aweary Jan 25, 2017

jquense commented Jan 25, 2017

aweary Jan 25, 2017

aweary commented Jan 25, 2017

jquense commented Jan 25, 2017 •

edited

Loading

nhunzaker commented Jan 25, 2017

gaearon commented Jan 25, 2017

nhunzaker commented Jan 25, 2017

gaearon commented Jan 25, 2017

nhunzaker commented Jan 25, 2017

aweary commented Jan 25, 2017

aweary commented Jan 25, 2017

gaearon commented Jan 25, 2017

jquense commented Jan 25, 2017

jquense commented Jan 25, 2017

aweary Jan 25, 2017

jquense Jan 25, 2017

aweary Jan 25, 2017 •

edited

Loading

nhunzaker Jan 31, 2017

nhunzaker Jan 31, 2017

jquense Feb 1, 2017

nhunzaker commented Feb 3, 2017

aweary commented Feb 13, 2017

nhunzaker left a comment

nhunzaker Feb 13, 2017

nhunzaker Feb 13, 2017

aweary Feb 13, 2017

nhunzaker Feb 13, 2017

jquense commented Feb 13, 2017

aweary left a comment

Add fixture components #8860

Add fixture components #8860

Conversation

jquense commented Jan 24, 2017 • edited Loading

jquense commented Jan 24, 2017

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

jquense commented Jan 25, 2017

Choose a reason for hiding this comment

aweary commented Jan 25, 2017

jquense commented Jan 25, 2017 • edited Loading

nhunzaker commented Jan 25, 2017

gaearon commented Jan 25, 2017

nhunzaker commented Jan 25, 2017

gaearon commented Jan 25, 2017

nhunzaker commented Jan 25, 2017

aweary commented Jan 25, 2017

aweary commented Jan 25, 2017

gaearon commented Jan 25, 2017

jquense commented Jan 25, 2017

jquense commented Jan 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aweary Jan 25, 2017 • 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

nhunzaker commented Feb 3, 2017

aweary commented Feb 13, 2017

nhunzaker left a 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

jquense commented Feb 13, 2017

aweary left a comment

Choose a reason for hiding this comment

jquense commented Jan 24, 2017 •

edited

Loading

jquense commented Jan 25, 2017 •

edited

Loading

aweary Jan 25, 2017 •

edited

Loading