test/encryption: differentiate identical test names #1102
Conversation
This should make test results less ambiguous when one of the tests fails.
There was a problem hiding this comment.
Just some thoughts on how the test names should change to be more informative!
These specific tests (and exhausting the worker or not) reminded me of something that will likely come up in the blob/s3 situation:
Client Audit attachments, which are stored as Blobs, get processed and put into a separate client_audits table, I think so it's easy to export a csv of all client audit events for a form across submissions. These client audits are processed by a worker, but if someone tries to export the data before the worker has come along, the processing will happen on the fly during the export.
| @@ -355,7 +355,7 @@ describe('managed encryption', () => { | |||
| result['media/testfile.jpg'].should.equal('hello this is a suffixed file'); | |||
| })))))); | |||
|
|
|||
| it('should decrypt client audit log attachments', testService((service, container) => | |||
| it('should decrypt client audit log attachments (1)', testService((service, container) => | |||
There was a problem hiding this comment.
This test looks like it's running exhaust(container) to process the client audits and that they are processed successfully without error. But since they are encrypted, the worker does not actually decrypt them, so the client_audits table has 0 rows.
Maybe a better name for the test is should successfully process encrypted client audit log attachments without decrypting?
There was a problem hiding this comment.
Thanks for closer reading of this PR!
should successfully process encrypted client audit log
Does it actually process them?! I'd expect entries in the client_audits table, but the test asserts there are none... Would a better title be: should mark encrypted client audit log attachments as processed, but not process them?
There was a problem hiding this comment.
Something like that! In lib/worker/submission.attachment.update.js I think it's skipping attachments of encrypted submissions. So it's "processing" the attachment event to check for client audits but actually ignoring them.
There was a problem hiding this comment.
should skip encrypted client audit log attachments and mark them as processed?
| @@ -376,7 +376,7 @@ describe('managed encryption', () => { | |||
| where action='submission.attachment.update' and processed is not null and failures = 0`) | |||
| .then((count) => { count.should.equal(4); }))))); | |||
|
|
|||
| it('should decrypt client audit log attachments', testService((service) => | |||
| it('should decrypt client audit log attachments (2)', testService((service) => | |||
There was a problem hiding this comment.
This test is more accurately about decrypting encrypted client audit attachments during export so I think the name can stay the same.
This should make test results less ambiguous when one of the tests fails.