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

Mocking for the field method of superagent #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maxkoryukov
Copy link
Contributor

@maxkoryukov maxkoryukov commented Feb 11, 2017

This is just a first step in implementation of mocking of the field function (see docs). This function is very useful when you are trying to send multipart form.

I just want to know, whether such improvements are interesting for you (as maintainers and owners)

Required for merge:

  • owner approvement
  • tests
  • ????

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.9%) to 95.082% when pulling 77116df on maxkoryukov:feature/mock-field-method into 6ff0027 on A:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-4.9%) to 95.082% when pulling 77116df on maxkoryukov:feature/mock-field-method into 6ff0027 on A:master.

@maxkoryukov
Copy link
Contributor Author

@A, @tbranyen could you take a look at this PR to confirm, that it is useful. If yes - I will add tests and will check for a unmock (I am not sure, how new feature works with mock reset)

And a question about design: should field-method save params to the body or not?

@A
Copy link
Owner

A commented Feb 14, 2017

I think it can be useful. But it's better to ask @tbranyen because he is the maintainer now. I believe him)

@tbranyen
Copy link
Collaborator

I'll look it over asap!

@tbranyen
Copy link
Collaborator

Looks good to me. Seems useful!

@maxkoryukov
Copy link
Contributor Author

I will add tests;)

@maxkoryukov
Copy link
Contributor Author

sorry... only on the next week
but we are using your package for testing, so I am forced to make this PR merge-able ;)

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.

None yet

4 participants