Skip to content
This repository has been archived by the owner on Jan 7, 2024. It is now read-only.

Cassette generation is a blocking task #128

Closed
sssoleileraaa opened this issue Aug 20, 2020 · 0 comments · Fixed by #129
Closed

Cassette generation is a blocking task #128

sssoleileraaa opened this issue Aug 20, 2020 · 0 comments · Fixed by #129

Comments

@sssoleileraaa
Copy link
Contributor

Description

Say you want to update the sdk like so:

--- a/sdclientapi/__init__.py
+++ b/sdclientapi/__init__.py
@@ -103,8 +103,8 @@ class API:
         self.token = None  # type: Optional[str]
         self.token_expiration = None  # type: Optional[datetime]
         self.token_journalist_uuid = None  # type: Optional[str]
-        self.journalist_first_name = None  # type: Optional[str]
-        self.journalist_last_name = None  # type: Optional[str]
+        self.first_name = None  # type: Optional[str]
+        self.last_name = None  # type: Optional[str]
         self.req_headers = dict()  # type: Dict[str, str]
         self.proxy = proxy  # type: bool
         self.default_request_timeout = default_request_timeout or DEFAULT_REQUEST_TIMEOUT
@@ -244,8 +244,8 @@ class API:
         self.token = token_data["token"]
         self.token_expiration = datetime.strptime(token_data["expiration"], "%Y-%m-%dT%H:%M:%S.%fZ")
         self.token_journalist_uuid = token_data["journalist_uuid"]
-        self.journalist_first_name = token_data["journalist_first_name"]
-        self.journalist_last_name = token_data["journalist_last_name"]
+        self.first_name = token_data["journalist_first_name"]
+        self.last_name = token_data["journalist_last_name"]
 
         self.update_auth_header()
diff --git a/tests/test_api.py b/tests/test_api.py
index b47a2aa..24f3e38 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -70,8 +70,8 @@ class TestAPI(unittest.TestCase):
         self.assertTrue(isinstance(self.api.token, str))
         self.assertTrue(isinstance(self.api.token_expiration, datetime.datetime))
         self.assertTrue(isinstance(self.api.token_journalist_uuid, str))
-        self.assertTrue(isinstance(self.api.journalist_first_name, (str, type(None))))
-        self.assertTrue(isinstance(self.api.journalist_last_name, (str, type(None))))
+        self.assertTrue(isinstance(self.api.first_name, (str, type(None))))
+        self.assertTrue(isinstance(self.api.last_name, (str, type(None))))
 
     @vcr.use_cassette("data/test-get-sources.yml")
     def test_get_sources(self):
diff --git a/tests/test_apiproxy.py b/tests/test_apiproxy.py
index f7ba35f..819ddfa 100644
--- a/tests/test_apiproxy.py
+++ b/tests/test_apiproxy.py
@@ -66,8 +66,8 @@ class TestAPIProxy(unittest.TestCase):
         self.assertTrue(isinstance(self.api.token, str))
         self.assertTrue(isinstance(self.api.token_expiration, datetime.datetime))
         self.assertTrue(isinstance(self.api.token_journalist_uuid, str))
-        self.assertTrue(isinstance(self.api.journalist_first_name, (str, type(None))))
-        self.assertTrue(isinstance(self.api.journalist_last_name, (str, type(None))))
+        self.assertTrue(isinstance(self.api.first_name, (str, type(None))))
+        self.assertTrue(isinstance(self.api.last_name, (str, type(None))))
 
     @dastollervey_datasaver
     def test_get_sources(self):

How would you regenerate cassettes used by functional tests? Once you set up your appvm to run the dev server and proxy and to be able to talk to the sd-sdk functional tests, then what do you do? The docs say:

Now, delete any related JSON file under data/ directory, or remove all of them, and then execute make test TEST=tests/test_apiproxy.py. This is command will generate the new data files, which can be used in CI or any other system.

I think what's missing here is that you actually need to first delete data/setUp.json and comment out

@dastollervey_datasaver
and also you have to do something in order to regenerate the new cassette for test_api.py (not just for test_apiproxy.py) which uses data/test-setup.yaml which fails when you delete it. This is still unclear.

Also say you want to make a change such as:

diff --git a/sdclientapi/sdlocalobjects.py b/sdclientapi/sdlocalobjects.py
index 5e5443b..f0525e5 100644
--- a/sdclientapi/sdlocalobjects.py
+++ b/sdclientapi/sdlocalobjects.py
@@ -59,8 +59,10 @@ class Reply:
 
     def __init__(self, **kwargs) -> None:  # type: ignore
         self.filename = ""  # type: str
-        self.journalist_username = ""  # type: str
         self.journalist_uuid = ""  # type: str
+        self.journalist_username = ""  # type: str
+        self.journalist_first_name = ""  # type: str
+        self.journalist_last_name = ""  # type: str
         self.is_deleted_by_source = False  # type: bool
         self.reply_url = ""  # type: str
         self.size = 0  # type: int
@@ -76,8 +78,10 @@ class Reply:
 
         for key in [
             "filename",
-            "journalist_username",
             "journalist_uuid",
+            "journalist_username",
+            "journalist_first_name",
+            "journalist_last_name",
             "is_deleted_by_source",
             "reply_url",
             "size",

You would need to make sure not to restart the server so you can use the same token right? This needs to be clarified in the docs.

These are two examples of changes that are currently being used successfully in the securedrop-client pr here: freedomofpress/securedrop-client#1137

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant