issue #1362 Upgrade to openpgp.js v5

[IvanPizhenko]

This PR upgrades OpenPGP.js to v 5.1.0

close #1362

---

Tests (delete all except exactly one):

• Tests added or updated

---

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

[IvanPizhenko]

I understand this, but if you at least knew what to lookup, I had no idea completely what can be done else. So I would (and I already said this before) stick for some time with such slightly patched openpgp.js and would wait until they fix web-stream-tools so that it does not require any extra steps to use it and then update to them.

[tomholub]

Code looks good except the tests as below.

I've closed all conversations that can be closed, and left some open to resolve (you need to click a few times to show them all since there is so many).

Here is a screenshot - this is still unresolved, I cannot merge such large changes together with test asset changes, regardless how small the test asset changes are. Tests must be exactly testing same inputs and same outputs. You can change the inputs in another PR, but not in this one - please revert.

[tomholub]

Meanwhile @rrrooommmaaa please have a look at this. We'll soon have to update browser-extension to OpenPGP v5 as well - your input on this PR will be greatly appreciated 👍 (see https://github.com/FlowCrypt/flowcrypt-ios/invitations - I wanted to request a review from you but GH doesn't allow until you are added to the repo).

[IvanPizhenko]

@tomholub Please read this feedback from openpgp.js developers regarding capabilities - openpgpjs/openpgpjs#1493 (comment)
Anyway, what I did is tried to port original code of getExpirationTime() with capabilities.

[IvanPizhenko]

Code looks good except the tests as below.

I've closed all conversations that can be closed, and left some open to resolve (you need to click a few times to show them all since there is so many).

Here is a screenshot - this is still unresolved, I cannot merge such large changes together with test asset changes, regardless how small the test asset changes are. Tests must be exactly testing same inputs and same outputs. You can change the inputs in another PR, but not in this one - please revert.

@tomholub In this case these tests will be failing and I will have to mark them with skip to have CI passing. Is it ok?

[tomholub]

@tomholub In this case these tests will be failing and I will have to mark them with skip to have CI passing. Is it ok?

Why would they be failing? If it's just removed comments and \r\n changed to \n? I was exactly worried about this - that there is something more going on.

What will they be failing on and why?

[IvanPizhenko]

@tomholub In this case these tests will be failing and I will have to mark them with skip to have CI passing. Is it ok?

Why would they be failing? If it's just removed comments and \r\n changed to \n? I was exactly worried about this - that there is something more going on.

What will they be failing on and why?

These changes to test string are done because strings returned by openpgp.js v5 are different. So I had to fix these strings to match what openpgp.js returns now. Because test matches strings exactly (is.deep.equal(...)).

[tomholub]

These changes to test string are done because strings returned by openpgp.js v5 are different. So I had to fix these strings to match what openpgp.js returns now. Because test matches strings exactly (is.deep.equal(...)).

My bad, I thought these were test inputs. Now I see these are outputs. Looks good.

[tomholub]

Once tests are passing, I'm good to merge it, assuming Roman is.

[tomholub]

@IvanPizhenko well done to update to v5, not an easy task.

[IvanPizhenko]

@tomholub I can see in CI that Appium UI tests fail. What's that?

[tomholub]

Likely unrelated. Try updating the branch to master, let's see if that passes.

[IvanPizhenko]

@tomholub Still failing, even after multiple times merging updates from master

[tomholub]

@IvanPizhenko let's have @sosnovsky finalize this, assuming the issue is unrelated to the code (which it should be, we haven't actually changed the app yet)