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

world spec #253

Closed
wants to merge 7 commits into from
Closed

world spec #253

wants to merge 7 commits into from

Conversation

dmaii
Copy link

@dmaii dmaii commented Feb 22, 2016

#137

I'm not sure if _renderMap is testable here without either exposing it or doing a non-shallow render(not advisable for unit tests).

We could also make this a separate react component with width/height as state, if it's reusable.

@jbenet jbenet added the status/deferred Conscious decision to pause or backlog label Feb 22, 2016
@dignifiedquire
Copy link
Member

Yeah _renderMap is a bit tricky, it does sound like a good idea to pull it into it's own component for ease of testing. Wanna do this in here?

it('should render an svg', () => {
const c = [[12, 14]]

const el = render(<WorldMap coordinates={c}/>)
Copy link
Author

Choose a reason for hiding this comment

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

I decided to do a static render here since this component only has one child.

const c = [[12, 14]]

const el = render(<WorldMap coordinates={c}/>)
expect(el.find('svg').length).to.equal(1)
Copy link
Author

Choose a reason for hiding this comment

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

I tried to think of more meaningful assertions to add here but drew a blank. The resulting output of _renderMap is basically all generated by d3 and I couldn't find anything else on the rendered output that could be used to assert correctness.

Copy link
Member

Choose a reason for hiding this comment

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

You could assert that the point is drawn that you passed in

@dmaii
Copy link
Author

dmaii commented Feb 23, 2016

💯

expect(el.find('svg').length).to.equal(1)
})

it('should have the correct coordiates', () => {
Copy link
Member

Choose a reason for hiding this comment

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

typo coordinates

@dignifiedquire
Copy link
Member

Thanks, one small note other than it looks good. Could you squash your commits together please?

@dmaii dmaii closed this Feb 23, 2016
@jbenet jbenet removed the status/deferred Conscious decision to pause or backlog label Feb 23, 2016
@dmaii dmaii mentioned this pull request Feb 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants