sendgrid / python-http-client

Twilio SendGrid's Python HTTP Client for calling APIs
https://sendgrid.com
MIT License
125 stars 101 forks source link

Test of date range in license file #121

Closed frenzymadness closed 5 years ago

frenzymadness commented 5 years ago

I would like to discuss whether is a good idea to have a test which is testing date range (Copyright (c) 2012 - 2019 SendGrid, Inc.). I understand the need described in the original issue (https://github.com/sendgrid/python-http-client/issues/57) but I don't think that it's a good idea to have a unit test for that.

HTTP client is packaged in various Linux distributions. Now imagine that you have packaged HTTP client version from 2018 and you want just a rebuild that package in 2019. It'll fail. Also, when you create a first pull request after a new year with some bugfix or a new feature, the tests will fail and it's not your fault.

Some workaround might be to change the date range in license file as a first thing every year but you probably don't want to release a new version and without a new version, nobody will update until they have to.

There is a PR to fix the problem now (https://github.com/sendgrid/python-http-client/pull/120) but I'd rather remove that test entirely. If it's really necessary to test it, it might be possible to move it to Travis config so it'd tested only in CI and not with unit tests.

What do you think?

thinkingserious commented 5 years ago

Hello @frenzymadness,

How about if we made that unit test throw a warning instead?

With Best Regards,

Elmer

frenzymadness commented 5 years ago

That's a good idea. We can write the test this way:

def test__daterange(self):
        try:
            self.assertIn(self.pattern, self.licensefile)
        except AssertionError:
            warnings.warn("License file does not contain a current year!")

which will produce this output in tox and doesn't mark tests as failed.

GLOB sdist-make: /home/lbalhar/Software/python-http-client/setup.py
py37 inst-nodeps: /home/lbalhar/Software/python-http-client/.tox/dist/python_http_client-3.1.0.zip
WARNING: Discarding $PYTHONPATH from environment, to override specify PYTHONPATH in 'passenv' in your configuration.
py37 installed: python-http-client==3.1.0
py37 run-test-pre: PYTHONHASHSEED='2907303746'
py37 runtests: commands[0] | /home/lbalhar/Software/python-http-client/.tox/py37/bin/python -m unittest discover -v
test__daterange (tests.test_daterange.DateRangeTest) ... /home/lbalhar/Software/python-http-client/tests/test_daterange.py:22: UserWarning: License file does not contain a current year!
  warnings.warn("License file does not contain a current year!")
ok
test_file_existence (tests.test_repofiles.RepoFiles) ... ok
test__ (tests.test_unit.TestClient) ... ok
test__build_client (tests.test_unit.TestClient) ... ok
test__build_url (tests.test_unit.TestClient) ... ok
test__build_versioned_url (tests.test_unit.TestClient) ... ok
test__getattr__ (tests.test_unit.TestClient) ... ok
test__init__ (tests.test_unit.TestClient) ... ok
test__update_headers (tests.test_unit.TestClient) ... ok
test_client_pickle_unpickle (tests.test_unit.TestClient) ... ok

----------------------------------------------------------------------
Ran 10 tests in 0.005s

OK
______________________________________________________ summary _______________________________________________________
  py37: commands succeeded
  congratulations :)

Does it look good to you?

thinkingserious commented 5 years ago

Nice! Thanks @frenzymadness!

Would you like to submit a PR?