Bug 206159 - [EWS] Add unit tests for factories
Summary: [EWS] Add unit tests for factories
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-12 17:26 PST by Aakash Jain
Modified: 2020-01-13 13:34 PST (History)
4 users (show)

See Also:


Attachments
Patch (9.10 KB, patch)
2020-01-12 17:35 PST, Aakash Jain
no flags Details | Formatted Diff | Diff
Patch (9.11 KB, patch)
2020-01-13 13:11 PST, Aakash Jain
jbedard: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2020-01-12 17:26:25 PST
Add unit tests for EWS factories.
Comment 1 Aakash Jain 2020-01-12 17:35:02 PST
Created attachment 387498 [details]
Patch
Comment 2 Alexey Proskuryakov 2020-01-12 17:44:12 PST
Comment on attachment 387498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387498&action=review

> Tools/ChangeLog:3
> +        [EWS] Add unit tests for factories

Will this run as part of an existing test suite on bots?
Comment 3 Aakash Jain 2020-01-12 17:53:11 PST
(In reply to Alexey Proskuryakov from comment #2)
> Will this run as part of an existing test suite on bots?
Yes, in fact these unit-tests were automatically run by the Services EWS for this patch as well (since runUnittests.py script automatically runs _unittest.py files in that directory).

The corresponding build for this patch run 180 tests (in https://ews-build.webkit.org/#/builders/20/builds/11417/steps/7/logs/stdio), while previously 172 tests were being run (e.g.: https://ews-build.webkit.org/#/builders/20/builds/11092/steps/7/logs/stdio).
Comment 4 Jonathan Bedard 2020-01-13 09:28:47 PST
Comment on attachment 387498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387498&action=review

>> Tools/ChangeLog:3
>> +        [EWS] Add unit tests for factories
> 
> Will this run as part of an existing test suite on bots?

Yes, Tools/BuildSlaveSupport/ews-build/runUnittests.py will pick it up.

> Tools/BuildSlaveSupport/ews-build/factories_unittest.py:40
> +        actual_steps = map(step_to_dict, actual_steps)

I would expect the style checker to want an empty line here to separate the function
Comment 5 Aakash Jain 2020-01-13 13:11:48 PST
Created attachment 387561 [details]
Patch
Comment 6 Aakash Jain 2020-01-13 13:14:42 PST
(In reply to Jonathan Bedard from comment #4)
> I would expect the style checker to want an empty line here to separate the function
Added empty line.
Comment 7 Aakash Jain 2020-01-13 13:33:06 PST
Committed r254448: <https://trac.webkit.org/changeset/254448>
Comment 8 Radar WebKit Bug Importer 2020-01-13 13:34:11 PST
<rdar://problem/58541484>