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: stop using unittest_toolbox in new tests #1792

Merged
merged 2 commits into from
Jan 25, 2022

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Jan 24, 2022

Fixes parts of #1745

Description of the changes being introduced by the pull request:

Update new test modules to stop using unittest_toolbox in preparation for its removal in #1790.

The tools provided by unittest_toolbox can easily (in a more obvious way) be replaced by using the standard library modules tempfile and random (no more used) directly.

In the case of tempdir and -file creation/removal, skipping the use of unittest_toolbox, which creates/removes a tmp dir for each test by default, also uncovers some test cleanup failures, which would occur when temporary test directories were removed while a test server hadn't released them.
(see except OSError: pass in unittest_toolbox's tearDown method)

Change details

test_fetcher_ng.py:

  • Stop implicitly creating (setUp) and removing (tearDown) tmp test dirs.
  • Move now manual creation of an exemplary targets file to setUpClass, as the same file is used by all tests. And remove it explicitly in tearDownClass after killing the server (see note about failure above).
  • Trigger URL parsing error with a hardcoded invalid URL string instead of a random string.

test_updater_ng.py

  • Stop implicitly creating (setUp) and removing (tearDown) tmp test dirs.
  • Explicitly create tmp test dirs in setUp, but don't remove them in tearDown to avoid above mentioned failures. They will be removed all at once when removing the tmp root test dir in tearDownClass

--

unrelated change: define a TESTS_DIR constant for convenience

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

Define TESTS_DIR constant in tests/util.py as full path to the
parent directory of the util module. This may be used to reliably
read other files in tests dir, such es "repository_data" or
"simple_server", regardless of cwd.

This commit also replaces a couple of `getcwd() + "filename"` with
`TESTS_DIR + filename`, so that in the future (post theupdateframework#1790) we
should be able to invoke the tests from anywhere, not only from
within the tests directory as is now the case.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Update new test modules to stop using unittest_toolbox, in
preparation for its removal in theupdateframework#1790.

The tools provided by unittest_toolbox can easily (in a more
obvious way) be replaced by using the standard library modules
`tempfile` and `random` (no more used) directly.

In the case of tempdir and -file creation/removal, skipping the use
of unittest_toolbox, which does this by default, also uncovers some
test cleanup failures, which would occur when temporary test
directories were removed while a test server hadn't released them.
(see `except OSError: pass` in unittest_toolbox's `tearDown`
method)

**Change details**

**test_fetcher_ng.py:**
- Stop implicitly creating (setUp) and removing (tearDown) tmp test
dirs.  -Move now manual creation of an exemplary targets file to
setUpClass, as the same file is used by all tests. And remove it
explicitly in tearDownClass after killing the server (see note
about failure above).  - Trigger URL parsing error with a hardcoded
invalid URL string instead of a random string.

**test_updater_ng.py**
- Stop implicitly creating (setUp) and removing (tearDown) tmp test
dirs.
- Explicitly create tmp test dirs in setUp, but don't remove
them in tearDown to avoid above mentioned failures. They will be
removed all at once when removing the tmp root test dir in
tearDownClass

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1739961338

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.692%

Totals Coverage Status
Change from base Build 1723684323: 0.0%
Covered Lines: 4089
Relevant Lines: 4170

💛 - Coveralls

@lukpueh lukpueh mentioned this pull request Jan 24, 2022
@lukpueh lukpueh requested a review from jku January 24, 2022 13:14
Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good idea to me: some test case helpers might be a good idea but Modified_TestCase has not been a positive influence in our test code for some time...

cls.file_contents = b"junk data"
cls.file_length = len(cls.file_contents)
with tempfile.NamedTemporaryFile(
dir=os.getcwd(), delete=False
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't know you can close a tempfile and not delete it, cool.

Copy link
Collaborator

@MVrachev MVrachev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the changes!
They simplify the code a lot!
LGTM!

@lukpueh lukpueh merged commit f28d45c into theupdateframework:develop Jan 25, 2022
lukpueh added a commit to lukpueh/tuf that referenced this pull request Jan 25, 2022
re https:
  --> `https` is handled transparently by `requests` library used in default
  request fetcher. no need to test in python-tuf

re toolbox: theupdateframework#1792

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh added a commit to lukpueh/tuf that referenced this pull request Jan 26, 2022
Remove tests for legacy client, repository/developer tool and
command line tools, which will be removed in subsequent commits.

This commits also removes obsolete test tooling:
- Regarding simple_https_server + test certificates -- http/https
is no longer handled by tuf client directly but transparently by
the underlying requests module used by the default fetcher
implementation.
- For details about unittest_toolbox see theupdateframework#1792

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
MVrachev pushed a commit to MVrachev/tuf that referenced this pull request Feb 7, 2022
Remove tests for legacy client, repository/developer tool and
command line tools, which will be removed in subsequent commits.

This commits also removes obsolete test tooling:
- Regarding simple_https_server + test certificates -- http/https
is no longer handled by tuf client directly but transparently by
the underlying requests module used by the default fetcher
implementation.
- For details about unittest_toolbox see theupdateframework#1792

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
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.

4 participants