Refactor functional test fixtures in source timeout and metadata tests #6307
Conversation
nabla-c0d3
force-pushed
the
refactor-source-functional-tests-timeout-metadata-notfound
branch
from
22cdf95 to
a0c6e68
Compare
Codecov Report
@@ Coverage Diff @@
## develop #6307 +/- ##
===========================================
- Coverage 83.99% 83.98% -0.02%
===========================================
Files 61 61
Lines 4293 4301 +8
Branches 521 522 +1
===========================================
+ Hits 3606 3612 +6
- Misses 564 565 +1
- Partials 123 124 +1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
nabla-c0d3
force-pushed
the
refactor-source-functional-tests-timeout-metadata-notfound
branch
2 times, most recently
from
4f50a17 to
ce09086
Compare
nabla-c0d3
force-pushed
the
refactor-source-functional-tests-timeout-metadata-notfound
branch
from
ce09086 to
e6166d5
Compare
|
|
||
| Can be used for tests and utility scripts that need to add data to the DB outside of the | ||
| context of the source & journalist Flask applications. | ||
| """ |
There was a problem hiding this comment.
This could also eventually be used in utility scripts (to not have to instantiate a fake Flask app just to get "raw" access to the DB).
| SECUREDROP_DATA_ROOT: Path, | ||
| SESSION_EXPIRATION_MINUTES: float = 120, | ||
| NOUNS: Path = DEFAULT_SECUREDROP_ROOT / "dictionaries" / "nouns.txt", | ||
| ADJECTIVES: Path = DEFAULT_SECUREDROP_ROOT / "dictionaries" / "adjectives.txt", |
There was a problem hiding this comment.
These arguments allow configuring the SD config that gets generated.
| @@ -0,0 +1,79 @@ | |||
| from pathlib import Path | |||
There was a problem hiding this comment.
This file contains two factories, which follow the API convention of the factoryboy library (in case this library is ever added to the unit tests dependencies). The factories can later be used in more code locations.
nabla-c0d3
force-pushed
the
refactor-source-functional-tests-timeout-metadata-notfound
branch
from
e6166d5 to
6cca52f
Compare
| @@ -0,0 +1,140 @@ | |||
| from contextlib import contextmanager | |||
There was a problem hiding this comment.
Most of this is existing code taken from FunctionalTest.
| def spawn_sd_servers( | ||
| config_to_use: SecureDropConfig | ||
| ) -> Generator[SdServersFixtureResult, None, None]: | ||
| """Spawn the source and journalist apps as separate processes with the supplied config.""" |
There was a problem hiding this comment.
The above is the "killer" feature of the new spawn_sd_servers fixture and other factory code: the source and journalist apps to be spawned will use the supplied config_to_use regardless of the content of securedrop/config.py.
The confusion that comes from having to deal with live SDConfig objects that don't match the content of securedrop/config.py has been one of the biggest challenges for me when updating the test suite. This also comes from having different parts of the code sometimes use the live object or sometimes securedrop/config.py (it also depends on the order of imports in the code base...).
I think one medium term goal (started with some of the changes here) is to get rid of securedrop/config.py in the context of the test suite.
|
This should be ready for review again. Thanks! |
| @@ -0,0 +1,252 @@ | |||
| import time | |||
There was a problem hiding this comment.
Most of this is existing code taken from FunctionalTest but "repackaged" as separate, smaller/simpler classes.
Refactor source app warning tests using new fixtures Refactor source app metadata test using new fixtures
nabla-c0d3
force-pushed
the
refactor-source-functional-tests-timeout-metadata-notfound
branch
from
6cca52f to
c407a44
Compare
| @@ -1,17 +0,0 @@ | |||
| from . import source_navigation_steps | |||
There was a problem hiding this comment.
Removed because it was duplicating TestSourceSessionLayout.
| @@ -1,10 +0,0 @@ | |||
| from . import source_navigation_steps | |||
There was a problem hiding this comment.
Removed because it was duplicating TestSourceLayout.test_notfound().
|
@legoktm Thanks for looking at this again; I removed the two duplicate tests. |
Status
Ready for review
Description of Changes
This PR includes and replaces the changes in #6207, and also addresses @legoktm 's feedback, such as putting the classes and functions in their "final" location in the code base.
Besides this addition, the intent and goals of the changes haven't changed, and are described there in details.