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

Add tests for Send.py #476

Merged
merged 6 commits into from
Nov 17, 2017
Merged

Add tests for Send.py #476

merged 6 commits into from
Nov 17, 2017

Conversation

artiemq
Copy link
Contributor

@artiemq artiemq commented Oct 28, 2017

Fixes #417

Checklist

  • [*] I have read the [Contribution Guide] and my PR follows them.
  • [*] My branch is up-to-date with the master branch.
  • [*] I have added tests that prove my fix is effective or that my feature works
  • [*] I have added necessary documentation (if appropriate)
  • [*] All the functions created/modified in this PR contain relevant docstrings.

Changes proposed in this pull request:

  • Add test for Send in send.py
  • Refactor Send.test_payload

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 28, 2017
@codecov-io
Copy link

codecov-io commented Oct 28, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@a403813). Click here to learn what that means.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #476   +/-   ##
=========================================
  Coverage          ?   85.58%           
=========================================
  Files             ?       30           
  Lines             ?     1027           
  Branches          ?      161           
=========================================
  Hits              ?      879           
  Misses            ?       57           
  Partials          ?       91
Impacted Files Coverage Δ
sendgrid/helpers/inbound/send.py 93.54% <87.5%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a403813...eb41ba5. Read the comment docs.

@artiemq
Copy link
Contributor Author

artiemq commented Oct 30, 2017

I tried to merge master into my branch and resolve conflicts but something went wrong.
Looks like travis declined build request

@mbernier @thinkingserious could you please review my PR?

@thinkingserious
Copy link
Contributor

@artiemq

We have not been able to merge your Pull Request, but because you are awesome - we wanted to make sure you could still get a SendGrid Hacktoberfest shirt.

Please go fill out our swag form before Nov 5th and we will send the shirt! (We know that you might have tried this before and it didn’t work, sorry about that!)

You have till Nov 5th to fill out this form in order to get the Hacktoberfest shirt!

Thank you for contributing during Hacktoberfest! We hope to see you in the repos soon! Just so you know, we always give away a SendGrid shirt for your first ever non-Hacktoberfest PR that gets merged.

@mbernier
Copy link
Contributor

Oh wow, that is super weird. I am going to see if I can get travis to kick over a build for us. WTF Travis!?

@mbernier
Copy link
Contributor

I just merged sendgrid-python master, this caused Travis to show up.


If you are interested, we would love the opportunity to talk to you about Hacktoberfest and our API libraries.

Our agenda would be: Explore what you liked and is there anything we can do to improve?

You can grab a time on my calendar that works for you and we can have a chat on Google Hangout or Skype. If you prefer, you can email me using my GitHub username at my company’s domain.

Thank you so much,

Matt Bernier - @mbernier - SendGrid Developer Experience Product Manager
Elmer Thomas - @thinkingserious - SendGrid Developer Experience Engineer
@sendgrid

@artiemq
Copy link
Contributor Author

artiemq commented Nov 17, 2017

Those tests failed.

# ./Docker or docker/Docker
def test_docker_dir(self):
self.assertTrue(os.path.isdir("./docker/Dockerfile"))
# ./docker-compose.yml or ./docker/docker-compose.yml
def test_docker_compose(self):
self.assertTrue(os.path.isfile('docker-compose.yml'))
# ./.env_sample
def test_env(self):
self.assertTrue(os.path.isfile('./env_sample'))

Looks like docker-compose.yml, env_sample are absent in master and ./docker/Dockerfile not a directory.

I think it would be better to comment failed tests if you want to use them later.

@mbernier
Copy link
Contributor

Those file tests failing is expected behavior, because we have PRs coming that will resolve them.

Thanks so much for following up!

Please do grab a time on my calendar (see previous comment)

Thanks!!

@mbernier mbernier merged commit b847377 into sendgrid:master Nov 17, 2017
@thinkingserious
Copy link
Contributor

Hello @artiemq,

Thanks again for the PR!

We appreciate your contribution and look forward to continued collaboration. Thanks!

Team SendGrid DX

@artiemq artiemq deleted the issue_417 branch November 17, 2017 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for Send.py
5 participants