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

Test of date range in license file #121

Closed
frenzymadness opened this issue Feb 7, 2019 · 3 comments · Fixed by #122
Closed

Test of date range in license file #121

frenzymadness opened this issue Feb 7, 2019 · 3 comments · Fixed by #122
Labels
difficulty: unknown or n/a fix is unknown in difficulty status: waiting for feedback waiting for feedback from the submitter type: question question directed at the library

Comments

@frenzymadness
Copy link
Contributor

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 (#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 (#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 thinkingserious added type: question question directed at the library difficulty: unknown or n/a fix is unknown in difficulty status: waiting for feedback waiting for feedback from the submitter labels Feb 11, 2019
@thinkingserious
Copy link
Contributor

Hello @frenzymadness,

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

With Best Regards,

Elmer

@frenzymadness
Copy link
Contributor Author

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
Copy link
Contributor

Nice! Thanks @frenzymadness!

Would you like to submit a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: unknown or n/a fix is unknown in difficulty status: waiting for feedback waiting for feedback from the submitter type: question question directed at the library
Projects
None yet
2 participants