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 UI components #2337

Merged
merged 6 commits into from May 18, 2017

Conversation

Projects
None yet
3 participants
@MatthewShao
Contributor

MatthewShao commented May 16, 2017

  • ValueEditor
  • FlowTable
@duo51

duo51 approved these changes May 16, 2017

@MatthewShao

This comment has been minimized.

Show comment
Hide comment
@MatthewShao

MatthewShao May 16, 2017

Contributor

@mhils @cle1000 This ValueEdiitor.js involves some code to get the selection on different browser, so I have to mock the related functions in a somehow ugly way... If you guys have any better idea, please let me know. 😃

Contributor

MatthewShao commented May 16, 2017

@mhils @cle1000 This ValueEdiitor.js involves some code to get the selection on different browser, so I have to mock the related functions in a somehow ugly way... If you guys have any better idea, please let me know. 😃

MatthewShao added some commits May 17, 2017

[web] Add a TFlow class to js/ducks/tutils.js
Many Components requires a flow object when being rendered, so we put a TFlow
class here, currently has the minimize structure, only contains the
attributes we needed in the components to be tested.
@mhils

mhils approved these changes May 17, 2017

Looks great! I think there's no good way to test the ValueEditor in a particularly useful way, but your tests are good and provide us with coverage.

All comments below (except the last one) are super super nitpicky, this looks good to merge from my side. :)

@mhils

This comment has been minimized.

Show comment
Hide comment
@mhils

mhils May 17, 2017

Member

@duo51: I don't understand your empty review here and in another PR lately, care to elaborate?

Member

mhils commented May 17, 2017

@duo51: I don't understand your empty review here and in another PR lately, care to elaborate?

MatthewShao added some commits May 18, 2017

@mhils mhils merged commit a9e002a into mitmproxy:master May 18, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
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