Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Basic UI rendering tests #3743

Merged
merged 19 commits into from
Dec 9, 2016
Merged

Basic UI rendering tests #3743

merged 19 commits into from
Dec 9, 2016

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Dec 8, 2016

@jacogr jacogr added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M7-ui labels Dec 8, 2016
@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Dec 8, 2016
@derhuerst
Copy link
Contributor

While I think this is an important thing we need, the tests are, I'd say, almost trivial. I'd rather test if e.g. the component contains the tx hash as text and leave the tests "renders successfully" out.

@ngotchac
Copy link
Contributor

ngotchac commented Dec 9, 2016

I kind of agree with @derhuerst : a few basic tests to see how it works is fine, but we should have more useful ones. Good first step anyway ! 👍

@jacogr
Copy link
Contributor Author

jacogr commented Dec 9, 2016

The whole idea behind the issue (as logged) is to bring in basic smoketests, i.e. the component renders, no magic. (As per the discussion around Sep in js-dev around the issue - https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/template/README.md#writing-tests - the gist is if you do nothing, at least do the "it renders" test)

From there it needs to be built up - since we have nothing, I would start with where we have bugs, replicate via test, fix it and let it grow organically. (Just pulled in a variety to not just do the simplest, but rather also cases where tied to Redux stores, bringing in context, etc.)

The most valuable place to spend time on test however would be (initially) on the MobX stores where most of the logic resides. (Also the only place where I found an issue thus far)

@ngotchac ngotchac added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 9, 2016
@gavofyork gavofyork merged commit 341777d into master Dec 9, 2016
@gavofyork gavofyork deleted the jg-test-ui branch December 9, 2016 22:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants