-
Notifications
You must be signed in to change notification settings - Fork 40
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
Release SecureDrop Client 0.7.0 #1465
Comments
[SDK Test Plan] Ensure support for common date string formats (#172)
Add journalist read receipts (#1417)
|
#1468 might be relevant to QA (and could be pulled into rc2 if repro'd but ro and I are still trying to repro ourselves). |
Here's a proposed update for the Test Plan for deletion changes, to make it a little more streamlined for QAers and also to add some stress testing. if @creviera you like this I'll update the issue. [edit: original issue updated] New deletion test plan belowSpeed up deletion of local files and database records (#1432, #1458)
Stress test local source deletion
Stress test local conversation deletion
(Added) No ghost replies
Regression testing: remote deletion
Test local deletion with network failures
|
@rocodes your changes look good to me, thanks for the update. One question though: shouldn't the following check be the first thing in the plan?
|
Note: I'm following @rocodes' test plan below, but I am not using the Speed up deletion of local files and database records (#1432, #1458)
Stress test local source deletion
Stress test local conversation deletion
Ghost issue with deleted sources❌ However, during entire source deletion, I'm observing the "ghost" problem with deleted sources re-appearing briefly on the next sync, then being cleaned up the following sync. I tried monitoring the |
Workstation: Qubes 4.0.4 I believe the test plan passes as written, but I've flagged some questions below. Per @eloquence in #1465 (comment), I'll plan to run through the stress-testing tomorrow. [SDK Test Plan] Ensure support for common date string formats (#172)
Add journalist read receipts (#1417)
👉 Yes, but: checkmarks are displayed (with a tooltip listing the sending user) while the reply is still pending. Is this the expected behavior? It surprises me to be able to have "seen" my own reply before it's sent.
Speed up deletion of local files and database records (#1432, #1458)
Migration
Test local deletion: happy path
Test local deletion right after conversation deletion
Test local deletion: network failure
Regression testing: remote deletion
Test local deletion: happy path
Test local deletion with network failures
Ensure no regression when deletion is done on the server
👉 Yes, but compare: Contents of
|
@creviera Yes - I kind of figured that all the QAers were updating from an existing setup and that the above tests would encompass the basic 'happy path' test, but yes we can add and make that explicit |
QAers: thank you for your detailed reports! @eloquence: Yikes, you're right about the DeletedSource; it looks like that change was clobbered and we didn't notice. Quick fix incoming. @cfm: You're right, the local deletion of files and messages removes the parent directory, whereas the server deletion does not (because it's simply updating the current messages, whether that means adding or removing). While neither case is 'wrong' (the directory will be re-created as soon as more files are submitted/downloaded, and since the source still remains in the source list, leaving the parent directory is not divulging any information in a dangerous way), it is an inconsistency, and I think we should fix it. Regarding seen-by behaviour, I agree that it's a little confusing to see the double checkmarks before a message is sent, and I think that's a gap in the way we specified the feature. I will investigate a little further. |
@cfm I might tap you for review of a couple very small PRs today since Allie is out, if that's ok with you. |
Stress test local source deletion
(Added) No ghost replies
Regression testing: remote deletion
No more ghosts on repeated deletions with the changes in #1473 present! 🎉 |
Workstation: Qubes 4.0.4 Retesting updated section of test plan: Speed up deletion of local files and database records (#1432, #1458)
Migration test
Stress test local source deletion
Stress test local conversation deletion
(Added) No ghost replies
Regression testing: remote deletion
Test local deletion with network failures
|
Test local deletion with network failures
Add feature to download all files from a single source (#1388)
Error with repeated "Download all" triggers while files are downloading.❗ The "Download all" feature was pretty resilient to my repeated attempts to break it. I observed that the feature stayed enabled while any file was still in "Downloading" state, so I tried clicking and hotkeying it repeatedly while files were already downloading, in the manner of a somewhat impatient user. :) In that scenario, I was once able to produce the following issue:
|
@eloquence Thank you for your thorough testing! I don't have much familiarity with the Download All code, but I wonder if this is related to our lack of a 'pause/stop download' feature. We should revisit this on Monday and decide if it's worth trying to pull in any changes to this feature. (I'm going to post a broader QA recap in a moment) |
Thanks Ro! I tried reproducing with a freshly initialized client and many downloads, and was not able to. With the debug log enabled, I can see that the duplicate jobs are detected and rejected. So this may have been a more sporadic edge case with some other timing issues in play. |
QA status and todos for Monday Approved, ready for merging and inclusion in rc2:
[update] Ready for review:
[update] Bug, but not a regression, suggest deferring fix til after the release: |
RC2 Test PlanMake sure to include the version(s) of the server that you are testing against. No double-check mark for a pending reply (#1470)
Selected source remains read in the source list after new message or reply comes in (#1464)
Selected source remains read in the source list after deleting conversation (#1464)
Client stress test during source deletion with fresh replies (#1468), No 'ghost sources' (#1473)
Global mouse selection cleared after login (#1477)
|
@rocodes - We have some time until rc2 is released, so the new test plan above just needs a review at some point plz |
QA Testers, @eloquence and @cfm: the new test plan is here: #1465 (comment) and rc2 can be downloaded and tested directly from https://github.com/freedomofpress/securedrop-dev-packages-lfs/blob/main/workstation/buster/securedrop-client_0.7.0-rc2%2Bbuster_all.deb (we're still testing some new changes that make it so that nightlies don't clobber our release candidate packages, so for now, just dnf install it directly in sd-app, and will make this better next time. |
It looks like the new rc2 deb is not getting picked up and added to |
Thanks @creviera. For reference, the process I followed to install the package:
No double-check mark for a pending reply (#1470)
Selected source remains read in the source list after new message or reply comes in (#1464)
I temporarily see "Message not yet available" preview text in bold, once it is decrypted, it is marked as read. That behavior seems fine to me, and even helpful to highlight new stuff arriving, but just noting it in case it's not what's desired. Selected source remains read in the source list after deleting conversation (#1464)
❗ Other testers, please observe the behavior when new sources are added on the server. Does the client register them as unread? I believe I observed one instance where newly created sources were marked as read, but I was not able to reproduce it - it may have been an observation error. Client stress test during source deletion with fresh replies (#1468), No 'ghost sources' (#1473)
(Tested with about 10 sources) Global mouse selection cleared after login (#1477)
|
This is indeed the desired behavior. The issue I was seeing earlier was around the selected source remaining bold until the next sync, so I'll update the STR linked to in the test plan to be clearer. I think we should also get @tina-ux's feedback on this behavior post-release. |
RC2 Test PlanMake sure to include the version(s) of the server that you are testing against. No double-check mark for a pending reply (#1470)
Selected source remains read in the source list after new message or reply comes in (#1464)
Selected source remains read in the source list after deleting conversation (#1464)
❌ Note: the behaviour differs between this case and the previous and I think there's a small bug here-- if I safe-delete messages and then reply to a source while it is still selected, the "Message not yet available" text is not bold. STR (cc @eloquence -- it's as you described!):
Client stress test during source deletion with fresh replies (#1468), No 'ghost sources' (#1473)
Global mouse selection cleared after login (#1477)
|
That does sound like a bug, @rocodes. The behavior should at least be consistent. I'll take a look to see why the style might not be getting applied as we expect. |
Workstation: Qubes 4.0.4 No double-check mark for a pending reply (#1470)
👉 Clarification: While the reply is pending:
Selected source remains read in the source list after new message or reply comes in (#1464)
Selected source remains read in the source list after deleting conversation (#1464)
❌ Yes, but regression:Steps to reproduce
Expected behaviorAfter the sync, source A is marked unread (bold) while the new message is downloaded. Actual behaviorAfter the sync, source A remains marked read (not bolded). Client stress test during source deletion with fresh replies (#1468), No 'ghost sources' (#1473)
Global mouse selection cleared after login (#1477)
|
^ Now that rc3 is merged, it can be QA'd with the following test plan: RC3 Test PlanFollow regression outlined here: #1465 (comment)
Also look for the ❌ in this QA report and try to reproduce the finding: #1465 (comment)
While you're at it, feel free to make notes of any unexpected behavior of read/unread (in addition to normal regression testing) since we'll be working on it in the near future to fix bugs like: #1463 |
Unfortunately, I still see the issue with new messages being marked as read, which I've filed as #1482. I don't see the behavior Cory described in #1463 (comment) - the "All files and messages deleted for this source" text is displayed unbolded. (I've verified that the revert was correctly applied in the version of the code included in the .deb.) |
From my perspective, since we've established that #1482 has been present at least since 0.6.0 and probably longer (possibly since the seen/unseen feature was introduced), and the test plan looks good otherwise, we're good to proceed with releasing SecureDrop Client 0.7.0; then we can debug some of the edge cases we're hitting here at a more relaxed pace. |
Also, the rc2 change, that we reverted in rc3, fixes a different issue than described in #1482, but since it's so closely related I think it's worth keeping out of the final release. No need to revert the revert. |
RC3 Test PlanQubes 4.0.4, staging @ 2.3.1, sd-app @ 0.7.0-rc3 Follow regression outlined here: #1465 (comment)
Also look for the ❌ in this QA report and try to reproduce the finding: #1465 (comment)
|
Description
This release includes using the latest version of the SDK: freedomofpress/securedrop-builder#299 and is part of a SDW release set.
This issue tracks the SecureDrop Client release 0.7.0. It will be organized by:
This release includes the following changes:
SecureDrop maintainers and testers: As you QA this release, please report back your testing results as comments on this ticket. File GitHub issues for any problems found, tag them "QA: Release", and associate them with the release milestone for tracking (or ask a maintainer to do so).
Updated Test plan
[Make sure to include the version(s) of the server that need to be tested against. If the release is being coordinated with a server release, specify rc versions of the server that need to be tested and release order. Once completed, insert or link to a test plan here. It can be left out until then.]
[SDK Test Plan] Ensure support for common date string formats (#172)
Add journalist read receipts (#1417)
Speed up deletion of local files and database records (#1432, #1458)
Migration test
Stress test local source deletion
loaddata.py
script and add > 10 sources.~/.securedrop_client/data/
directory; source's subdirectory is removed and all files are deletedsources
,messages
,replies
,draftreplies
, andfiles
tables for rows with the sourceid
and ensure that no matching rows are presentStress test local conversation deletion
loaddata.py
script and add > 10 sources.~/.securedrop_client/data/
directory; source's subdirectories should be removed and all relevant files deleted.sources
table and note the source's id (not UUID). Query themessages
,replies
,draftreplies
, andfiles
tables for rows with the sourceid
and ensure that no matching rows are present.(Added) No ghost replies
Regression testing: remote deletion
data
directory.~/.securedrop_client/data/
directory.Test local deletion with network failures
~/.securedrop_client/data/
none
or issuingsudo service tor stop
in the sd-whonix terminal.~/.securedrop_client/data/
directory; the source's subdirectory should be removed and all relevant files deleted.sources
table and note the source's id and UUID. Query themessages
,replies
,draftreplies
, andfiles
tables for rows with the sourceid
and ensure that no matching rows are present.uuid
is present.messages
,replies
,draftreplies
, andfiles
tables for rows with the sourceid
and ensure that no matching rows are present.Add feature to download all files from a single source (#1388)
Release tasks
sd-app
's template)export LANG=es_ES.utf-8; dpkg-reconfigure locales
), run the Client, and confirm that the application is translated.The text was updated successfully, but these errors were encountered: