1596114 - Migrate server unit tests to async/await

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P3)

Description

[Brian Hackett [Laid off!]]

A lot of server unit tests use somewhat complicated promise based logic for performing their actions. Migrating them to async/await would make them easier to read and maintain.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the report Brian!

What specific tests do you have in mind?
Should this be a meta?

Honza

Comment 2

[Brian Hackett [Laid off!]]

These are the tests in devtools/server/tests/unit. This issue came up in bug 1512152 in regard to thread front based tests in that directory, though the issue is more general than those tests specifically.

Comment 3

[Jim Blandy :jimb]

This'd be so awesome.

Comment 4

[Chujun Lu]

I'd like to take a stab at it.

My understanding of the task is to refactor the tests that use new Promise or use threadFrontTestFinished. Is that correct? May I submit a patch with some tests that I refactored to make sure I'm on the right track? Thanks! I might break this into more than one patches.

Comment 5

[Chujun Lu]

Attachment: Bug 1596114 - Refactor server unit tests part1 r=bhackett

I refactored some of the unit tests to see if I'm on the right direction. If so, I'll continue with other unit tests.

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/integration/autoland/rev/d74eec46c4ea1c3d7f287c359b51404425894ec5

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/d74eec46c4ea

Comment 8

[Chujun Lu]

Attachment: Bug 1596114 - Refactor server unit tests to async/await part2 r=jlast

The 3 test_objectgrips-fn-apply tests and 3 test_objectgrips-property-value tests used somewhat complicated callbacks returning promise . I thought they were not easy to read, and I might fail to understand its subtleness. I wondered why the tests were written in that way?

Comment 9

[Chujun Lu]

Attachment: Bug 1596114 - Refactor server unit tests part3 r=bhackett,jlast

Remove threadFrontTestFinished

Comment 10

[Chujun Lu]

Attachment: Bug 1596114 - Refactor server unit tests part4 r=bhackett,jlast

Remove threadFrontTestFinished

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49ecd149cade
Refactor server unit tests part4 r=bhackett,jlast

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f25ba905dedd
Refactor server unit tests part3 r=jlast,bhackett

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/49ecd149cade
https://hg.mozilla.org/mozilla-central/rev/f25ba905dedd

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c5a5a341613b
Refactor server unit tests to async/await part2 r=jlast

Comment 15

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/c5a5a341613b

Comment 16

[Chujun Lu]

Attachment: Bug 1596114 - Refactor server unit tests part5 r=bhackett,jlast

test_breakpoint-25.js is wip.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/826012956299
Refactor server unit tests part5 r=bhackett,jlast

Comment 18

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/826012956299

Comment 19

[Chujun Lu]

Attachment: Bug 1596114 - Refactor server unit tests part6 r=jlast

Remove threadFrontTestFinished

Comment 20

[Chujun Lu]

Attachment: Bug 1596114 - Refactor server unit tests part7 r=jlast

This should be the last patch for refactoring server xpcshell test. Thanks for the code reviews all the way. I would suggest reviewing and merging part6 first, since part7 removes waitForFinish, an option that part6 test files currently use.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ad8134a2621
Refactor server unit tests part6 r=jlast

Comment 22

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/1ad8134a2621