Ensure a wrap / unwrap tests run in the same order

[javifernandez]

This PR changes replaces the Promise based wrap / unwrap tests with an async/await approach. This allows us have a more consistent expectations for all the test cases and reduce the chances of flakiness.

This is specially important for WebKit, which compares the text output of the test against a pre-defined expected.txt.

There shouldn't be any functional change in the test cases, neither additions or removals.

[twiss]

I'll try; I just wanted to made the minimum changes to this already complex test, but I'll try. I agree this "isPossible" approach of creating "empty" Promises is ugly.

Thanks for taking a stab at this! However, it seems this refactoring indeed broke the tests, as all of them are missing now (here and here).

I think you need to make testWrapping async and await it in the setup:

[javifernandez]

Last version of the patch is worth giving it another review. I'd appreciate feedback on the 'vector.js' file, which contains the keys to be imported. Perhaps there are better data to use for this test. The keys defined there were copied form the import_export tests.

I tried to make all the code in the actual test-cases become async / await, but in order to avoid slowing down an already quite slow tests, I still used the Promise based approach for the setup phase. In any case, any suggestion to improve the code is really welcome. It's still quite difficult to understand how the test cases are being generated.

Regarding the test cases, it was in the original test the idea of ignoring the ones associated with unsupported algorithms. The problem is that it also discards test cases that fail in the import / export operations performed during the setup phase. This is the reason why the number of tests cases is different between browsers (eg gecko 1932396 and webkit 282578) This would not look nice in the wpt.fyi site, for instance.

[twiss]

Some naming nits below, but mostly it looks good to me.

The browser-specific skipped tests will all correspond to a test failure in the import key tests, right? If so it sounds fine to me, at least no bugs will be missed due to this.

[javifernandez]

Some naming nits below, but mostly it looks good to me.

The browser-specific skipped tests will all correspond to a test failure in the import key tests, right? If so it sounds fine to me, at least no bugs will be missed due to this.

Yes, most, if not all, related to bugs already reported. The problem is that this test wouldn't detect the specific failure, it'd just be missing. It could happen that if the import fails in all the browsers it wouldn't be noticed.

Additionally, the isWrappingPossible skips test cases that we may want to verify whether wrapping is possible or not. In general, I believe we should avoid generating different number of test cases; instead we should define them specifically with an associated expected result.

[twiss]

It could happen that if the import fails in all the browsers it wouldn't be noticed.

But even this would be noticed in the import tests, with a test failing for every browser, right?

Additionally, the isWrappingPossible skips test cases that we may want to verify whether wrapping is possible or not. In general, I believe we should avoid generating different number of test cases; instead we should define them specifically with an associated expected result.

On the one hand I agree; on the other hand, it seems kind of reasonable that if importing fails (which we already have a separate test failure for), then we can't test wrapping/unwrapping with it. So a missing test result seems kind of expected?

It might be bad if the import and wrapping/unwrapping tests ever get out of sync, though, such that a wrapping/unwrapping test could be skipped unexpectedly without a corresponding import test failure. Perhaps, to prevent that, we could reuse the same test vectors file between the import and wrapping/unwrapping test? (Though, I don't think it's critical and could also be done later, perhaps.)

[javifernandez]

It could happen that if the import fails in all the browsers it wouldn't be noticed.

But even this would be noticed in the import tests, with a test failing for every browser, right?

That's true. I think we may lack some additional tests for the import/export cases; I'm going to make sure we add them.

Additionally, the isWrappingPossible skips test cases that we may want to verify whether wrapping is possible or not. In general, I believe we should avoid generating different number of test cases; instead we should define them specifically with an associated expected result.

On the one hand I agree; on the other hand, it seems kind of reasonable that if importing fails (which we already have a separate test failure for), then we can't test wrapping/unwrapping with it. So a missing test result seems kind of expected?

It might be bad if the import and wrapping/unwrapping tests ever get out of sync, though, such that a wrapping/unwrapping test could be skipped unexpectedly without a corresponding import test failure. Perhaps, to prevent that, we could reuse the same test vectors file between the import and wrapping/unwrapping test? (Though, I don't think it's critical and could also be done later, perhaps.)

I also agree on this. Lets do it in a follow up patch.