Skip to content

Re-enable live gmail tests #3941

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Aug 31, 2021
Merged

Re-enable live gmail tests #3941

merged 7 commits into from
Aug 31, 2021

Conversation

limonte
Copy link

@limonte limonte commented Aug 29, 2021 •
edited
Loading

This PR re-enables 4 live gmail tests that were broken (#3929)

issue #3929

Tests (delete all except exactly one):

  • Does not need tests (refactor only, docs or internal changes)

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@limonte limonte changed the title [WIP] Re-enable live gmail tests Re-enable live gmail tests (part 1) Aug 30, 2021
@limonte limonte marked this pull request as ready for review August 30, 2021 10:43
@limonte limonte requested a review from rrrooommmaaa as a code owner August 30, 2021 10:43
@limonte
Copy link
Author

limonte commented Aug 30, 2021

@rrrooommmaaa In this PR I re-enabled 4 tests, there are still 5 of them left to re-enable, but that is blocked by this.

Let's review and merge this PR, and the rest of the tests I will re-enable in another one.

@limonte limonte marked this pull request as draft August 30, 2021 10:47
@limonte
Copy link
Author

limonte commented Aug 30, 2021

@rrrooommmaaa In this PR I re-enabled 4 tests, there are still 5 of them left to re-enable, but that is blocked by this.

Let's review and merge this PR, and the rest of the tests I will re-enable in another one.

Sorry, I just noticed the email from Tom which will help me to re-enable the rest of the tests. I will continue working on this PR.

@limonte limonte changed the title Re-enable live gmail tests (part 1) [WIP] Re-enable live gmail tests Aug 30, 2021
This was referenced Aug 30, 2021
Closed
Closed
@limonte
Copy link
Author

limonte commented Aug 30, 2021

Ready for review 🚀

  1. While re-enabling Thunderbird tests I faced 2 issues which are reported in Thunderbird signed + encrypted message: "..." instead of actual subject #3943 and Thunderbird signed + encrypted message - shows "Message Not Signed" in FlowCrypt [waiting for OpenPGP.js upgrade] #3944
  2. I merged two "Thunderbird [html]" tests into 1 for the sake of tests speed, those two tests were using the same message for testing.
  3. I merged two "Thunderbird [plain]" tests into 1 for the sake of tests speed, those two tests were using the same message for testing.

@limonte limonte changed the title [WIP] Re-enable live gmail tests Re-enable live gmail tests Aug 30, 2021
@limonte limonte marked this pull request as ready for review August 30, 2021 15:48
limonte
limonte commented Aug 30, 2021
View reviewed changes
extension/js/common/lang.ts
yourKeyCantOpenImportIfHave: 'Your current key cannot open this message. If you have any other keys available, you should import them now.\n',
encryptedCorrectlyFileBug: 'It\'s correctly encrypted for you. Please file a bug report if you see this on multiple messages. ',
singleSender: 'Normally, messages are encrypted for at least two people (sender and the receiver). It seems the sender encrypted this message manually for themselves, and forgot to add you as a receiver. This sometimes happens when the sender is using OpenPGP software other than FlowCrypt, because they have to configure encryption manually, and mistakes can happen.',
singleSender: 'Normally, messages are encrypted for at least two people (sender and the receiver). It seems the sender encrypted this message manually for themselves, and forgot to add you as a receiver. This sometimes happens when the sender is using OpenPGP software other than FlowCrypt, because they have to configure encryption manually, and mistakes can happen. ',
Copy link
Author

Choose a reason for hiding this comment

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

tiny fix for this:

CleanShot 2021-08-30 at 21 42 49@2x

@rrrooommmaaa
Copy link
Contributor

I guess some of lost messages may be present in exported-messages folder. It would be good to add these new Gmail messages there (or to another folder (or in EML format)) and add remarks in the tests with their message ids (file names) for easier restoration next time.

@limonte
Copy link
Author

limonte commented Aug 31, 2021 •
edited
Loading

I guess some of lost messages may be present in exported-messages folder. It would be good to add these new Gmail messages there (or to another folder (or in EML format)) and add remarks in the tests with their message ids (file names) for easier restoration next time.

Done, thanks 👍

I did backup in this format: .../exported-messages/live-gmail-test-emails/{EMAIL_SUBJECT} ({EMAIL_ID}).eml

@limonte
Copy link
Author

limonte commented Aug 31, 2021

Tests are failing with the reason that seems to be unrelated to the PR:

CleanShot 2021-08-31 at 15 17 57@2x

@rrrooommmaaa @martgil could that be related to #3942 even though it's not merged yet?

@rrrooommmaaa
Copy link
Contributor

rrrooommmaaa commented Aug 31, 2021 •
edited
Loading

yes, exported-messages are parsed automatically (JSONs files are expected to be there), so presence of a directory there makes it fail. Can you move this live-gmail... directory up one level, please?

@limonte
Copy link
Author

limonte commented Aug 31, 2021

yes, exported-messages are parsed automatically (JSONs files are expected to be there), so presence of a directory there makes it fail. Can you move this live-gmail... directory up one level, please?

Oh, didn't expect that, thanks for the hint!

@limonte
Copy link
Author

limonte commented Aug 31, 2021

All good now, ready for review 🚀

btw, this

  1. I merged two "Thunderbird [html]" tests into 1 for the sake of tests speed, those two tests were using the same message for testing.
  2. I merged two "Thunderbird [plain]" tests into 1 for the sake of tests speed, those two tests were using the same message for testing.

saved around 1 min of running time for live gmail tests

@rrrooommmaaa rrrooommmaaa merged commit 25205d6 into master Aug 31, 2021
@rrrooommmaaa rrrooommmaaa deleted the issue-3929-re-enable-live-gmail-tests branch August 31, 2021 14:58
@limonte limonte mentioned this pull request Aug 31, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@rrrooommmaaa rrrooommmaaa rrrooommmaaa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@limonte @rrrooommmaaa