After getting a +1 from everyone on the team, this PR removes the use of `codecov` from the Certbot repo because we keep having problems with it.
Two noteworthy things about this PR are:
1. I left the text at 4ea98d830b/.azure-pipelines/INSTALL.md (add-a-secret-variable-to-a-pipeline-like-codecov_token) because I think it's useful to document how to set up a secret variable in general.
2. I'm not sure what the text "Option -e makes sure we fail fast and don't submit to codecov." in `tox.cover.py` refers to but it seems incorrect since `-e` isn't accepted or used by the script so I just deleted the line.
As part of this, I said I'd open an issue to track setting up coveralls (which seems to be the only real alternative to codecov) which is at https://github.com/certbot/certbot/issues/7810.
With my change, failure output looks something like:
```
$ tox -e py27-cover
...
Name Stmts Miss Cover Missing
------------------------------------------------------------------------------------------
certbot/certbot/__init__.py 1 0 100%
certbot/certbot/_internal/__init__.py 0 0 100%
certbot/certbot/_internal/account.py 191 4 98% 62-63, 206, 337
...
certbot/tests/storage_test.py 530 0 100%
certbot/tests/util_test.py 374 29 92% 211-213, 480-484, 489-499, 504-511, 545-547, 552-554
------------------------------------------------------------------------------------------
TOTAL 14451 647 96%
Command '['/path/to/certbot/dir/.tox/py27-cover/bin/python', '-m', 'coverage', 'report', '--fail-under', '100', '--include', 'certbot/*', '--show-missing']' returned non-zero exit status 2
Test coverage on certbot did not meet threshold of 100%.
ERROR: InvocationError for command /Users/bmw/Development/certbot/certbot/.tox/py27-cover/bin/python tox.cover.py (exited with code 1)
_________________________________________________________________________________________________________________________________________________________ summary _________________________________________________________________________________________________________________________________________________________
ERROR: py27-cover: commands failed
```
I printed the exception just so we're not throwing away information.
I think it's also possible we fail for a reason other than the threshold not meeting the percentage, but I've personally never seen this, `coverage report` output is not being captured so hopefully that would inform devs if something else is going on, and saying something like "Test coverage probably did not..." seems like overkill to me personally.
* remove codecov
* remove unused variable group
* remove codecov.yml
* Improve tox.cover.py failure output.
Related to https://github.com/certbot/certbot/pull/7482, this removes some references to deprecated options in Certbot.
The only references I didn't remove were:
* In `certbot/tests/testdata/sample-renewal*` which contains a lot of old values and I think there's even some value in keeping them so we know if we make a change that suddenly causes old renewal configuration files to error.
* In the Apache and Nginx plugins and I created https://github.com/certbot/certbot/issues/7508 to resolve that issue.
If you run `mypy --platform darwin certbot/certbot/util.py` you'll get:
```
certbot/certbot/util.py:303: error: Name 'distro' is not defined
certbot/certbot/util.py:319: error: Name 'distro' is not defined
certbot/certbot/util.py:369: error: Name 'distro' is not defined
```
This is because mypy's logic for handling platform specific code is pretty simple and can't figure out what we're doing with `_USE_DISTRO` here. See https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks for more info.
Setting `_USE_DISTRO` to the result of `sys.platform.startswith('linux')` solves the problem without changing the overall behavior of our code here though.
This fixes part of https://github.com/certbot/certbot/issues/7803, but there's more work to be done on Windows.
I don't fully understand why, but since I updated my macbook to macOS Catalina, the test script currently fails to run for me with the versions of our dependencies we have pinned. Updating the dependencies solves the problem though and you can see Travis also successfully running tests with these new dependencies at https://travis-ci.com/certbot/certbot/builds/150573696.
I want to do what I did in https://github.com/certbot/certbot/pull/7733 to our Azure Pipelines setup, but unfortunately this isn't currently possible. The only filters available for service hooks for the "build completed" trigger are the pipeline and build status. See
To accomplish this, I propose splitting the "advanced" pipeline into two cases. One is for builds on protected branches where we want to be notified if they fail while the other is just used to manually run tests on certain branches.
* Refactor cli.py into a package with submodules
* Added unit tests for helpful module in cli.
* Fixed linter errors
* Fixed pylint issues
* Updated changelog.md
* Fixed test failing and mypy error. Appeared a new pylint error (seems to be in conflict with mypy)
mypy require zope.interface to be imported but when imported it is not used and pylint throws an error.
* Fixed pylint errors
* Apply changes to cli since last merge from master (efc8d49806)
* Fix lint
* Remaining lint errors
Co-authored-by: Adrien Ferrand <adferrand@users.noreply.github.com>
This fixes (part of) the problem identified in https://github.com/certbot/certbot/pull/7657#issuecomment-586506340.
When I tested our pylint setup on Python 3.5.9, 3.6.9, or 3.6.10, tests failed with:
```
************* Module acme.challenges
acme/acme/challenges.py:57:15: E1101: Instance of 'UnrecognizedChallenge' has no 'jobj' member (no-member)
************* Module acme.jws
acme/acme/jws.py:28:16: E1101: Class 'Signature' has no '_orig_slots' member (no-member)
```
These errors did not occur for me on Python 3.6.7 or Python 3.7+.
You also cannot run our lint setup on Python 2.7 because our pinned version of pylint's dependency `asteroid` does not support Python 2. Because of this, `pylint` is not installed in the virtual environment created by `tools/venv.py` and our [`lint` environment in tox specifies that Python 3 should be used](fd64c8c33b/tox.ini (L132)).
I tried updating pylint and its dependencies to fix the problem, but they still occur so I think adding back these disable checks on these lines again is the best fix for now.
As pylint is evolving, it improves its accuracy, and several pylint error suppression (`# pylint: disable=ERROR) added in certbot codebase months or years ago are not needed anymore to make it happy.
There is a (disabled by default) pylint error to detect the useless suppressions (pylint-ception: `useless-suppression`). It is not working perfectly (it has also false-positives ...) but it is a good start to clean the codebase.
This PR removes several of these useless suppressions as detected by the current pylint version we use.
* Remove useless suppress
* Remove useless lines
These tests failed at https://travis-ci.com/certbot/certbot/jobs/285202481 but do not include any output from the script about what went wrong because the string created from `subprocess.CalledProcessError` does not include value of output.
This PR fixes that by printing these values which `pytest` will include in the output if the test fails.
We should move ocsp.py to public API, as an upcoming OCSP prefetching functionality in Apache plugin relies on it, and as the plugins are note released in lockstep with the Certbot core, we need to be careful when changing those APIs.
* Move ocsp.py to public api
* Fix type annotations, move to pointing to an interface and fix linting
* Add certbot.ocsp to documentation table of contents
* Modify tests to reflect the changes in ocsp.py
* Add changelog entry
* Fix notAfter mock for tests
After a brief discussion in Mattermost, I shut down letsencrypt.readthedocs.io. Turns out we were linking to it in our README here so let's remove the broken link.
I didn't update the link to point to one of the readthedocs projects we still have because are main Certbot docs are self-hosted rather than being on readthedocs.
Currently if you go to https://certbot.eff.org/docs/api/certbot.crypto_util.html, there is a todo comment displayed at the top of the page. These todos were written for developers, not users, so I do not think they should be shown from our documentation.
This PR makes the quick and easy fix of configuring Sphinx not to show these todo items. I created #7752 to track removing all of these todos from our docstrings and disabling the Sphinx todo extension.
* Set todo_include_todos=False in sphinx-quickstart
* Remove todos from existing docs.
As discussed in #7539, we need proper tests of the Windows installer itself in order to variety that all the logic contained in a production-grade runtime of Certbot on Windows is correctly setup by each version of the installer, and so for a variety of Windows OSes.
This PR handles this requirement. The new `windows_installer_integration_tests` module in `certbot-ci` will:
* run the given Windows installer
* check that Certbot is properly installed and working
* check that the scheduled renew task is set up
* check that the scheduled task actually launch the Certbot renew logic
The Windows nightly tests are updated accordingly, in order to have the tests run on Windows Server 2012R2, 2016 and 2019.
These tests will evolve as we add more logic on the installer.
* Configure an integration test testing the windows installer
* Write the test module
* Configurable installer path, prepare azure pipelines
* Fix option
* Update test_main.py
* Add confirmation for this destructive test
* Use regex to validate certbot --version output
* Explicit dependency on a log output
* Use an exception to ask confirmation
* Use --allow-persistent-changes
