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

[WIP][web] Add tests for components/ContentView (con't) #2376

Merged
merged 11 commits into from Jun 12, 2017

Conversation

Projects
None yet
2 participants
@MatthewShao
Contributor

MatthewShao commented Jun 5, 2017

  • ContentLoader.jsx
  • ContentView.jsx
  • DownloadContentButton.jsx
  • MetaView.jsx
  • ShowFullContentButton.jsx
  • UploadContentButton.jsx
  • ViewSelector.jsx
@MatthewShao

This comment has been minimized.

Show comment
Hide comment
@MatthewShao

MatthewShao Jun 5, 2017

Contributor

@mhils If it looks good, please do not hurry to merge this PR, I would like to cover the whole ContentView directory and then merge. Giving it an approval would be welcome though.😄

Contributor

MatthewShao commented Jun 5, 2017

@mhils If it looks good, please do not hurry to merge this PR, I would like to cover the whole ContentView directory and then merge. Giving it an approval would be welcome though.😄

Show outdated Hide outdated web/src/js/__tests__/components/ContentView/ContentLoaderSpec.js
import TestUtils from 'react-dom/test-utils'
import mockXMLHttpRequest from 'mock-xmlhttprequest'
global.XMLHttpRequest = mockXMLHttpRequest

This comment has been minimized.

@mhils

mhils Jun 9, 2017

Member

I'm not sure if I understand this without a comment - which purpose does this serve here and in ContentViewSpec.js?

@mhils

mhils Jun 9, 2017

Member

I'm not sure if I understand this without a comment - which purpose does this serve here and in ContentViewSpec.js?

This comment has been minimized.

@MatthewShao

MatthewShao Jun 10, 2017

Contributor

Because we use XMLHttpRequest here:

// We use XMLHttpRequest instead of fetch() because fetch() is not (yet) abortable.

Introducing mock-xmlhttprequest dependency can save us a lot of troubles.

@MatthewShao

MatthewShao Jun 10, 2017

Contributor

Because we use XMLHttpRequest here:

// We use XMLHttpRequest instead of fetch() because fetch() is not (yet) abortable.

Introducing mock-xmlhttprequest dependency can save us a lot of troubles.

This comment has been minimized.

@mhils

mhils Jun 11, 2017

Member

Ok, but we never tell the mock function what to return, so what would happen if we didn't do this here?

@mhils

mhils Jun 11, 2017

Member

Ok, but we never tell the mock function what to return, so what would happen if we didn't do this here?

This comment has been minimized.

@MatthewShao

MatthewShao Jun 12, 2017

Contributor

If we don't mock XMLHttpRequest, it fails like this:
default

@MatthewShao

MatthewShao Jun 12, 2017

Contributor

If we don't mock XMLHttpRequest, it fails like this:
default

export default View => class extends React.Component {
export default function withContentLoader(View) {
return class extends React.Component {

This comment has been minimized.

@mhils

mhils Jun 12, 2017

Member

Good call on making this more clear! 👍

@mhils

mhils Jun 12, 2017

Member

Good call on making this more clear! 👍

@mhils

This comment has been minimized.

Show comment
Hide comment
@mhils

mhils Jun 12, 2017

Member

Awesome! :)

Member

mhils commented Jun 12, 2017

Awesome! :)

@mhils mhils merged commit c89d076 into mitmproxy:master Jun 12, 2017

4 checks passed

codecov/patch 96.87% of diff hit (target 86.91%)
Details
codecov/project 87.79% (+0.88%) compared to f3c2123
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment