Summary: | [build.webkit.org] Add unit-tests based on new buildbot | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||||||
Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aakash_jain, jbedard, ryanhaddad, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | Other | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=219364 https://bugs.webkit.org/show_bug.cgi?id=219374 |
||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 175056 | ||||||||||||
Attachments: |
|
Description
Aakash Jain
2020-11-30 11:45:08 PST
Created attachment 415044 [details]
WIP
(In reply to Aakash Jain from comment #1) > Created attachment 415044 [details] > WIP This is not final patch. Need https://bugs.webkit.org/show_bug.cgi?id=219364 to be landed first which will rename steps_unittest.py to steps_unittests_old.py Then this patch will add steps_unittest.py containing new unit-tests. Also we would be using existing runUnittests.py script (https://trac.webkit.org/browser/webkit/trunk/Tools/CISupport/ews-build/runUnittests.py) to run these unit-tests (both for EWS and build-webkit-org). Created attachment 415077 [details]
Patch
Created attachment 415151 [details]
Patch
(In reply to Aakash Jain from comment #4) > Created attachment 415151 [details] > Patch This patch ensures that the unit-tests run fine in Python 3. Comment on attachment 415077 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415077&action=review > Tools/CISupport/build-webkit-org/steps_unittest.py:199 > + timeout=1200, Since we're starting this from scratch, are we happy with this style? Or do we want: self.expectRemoteCommands( ExpectShell( workdir='wkdir', timeout=1200, logEnviron=True, logEnviron=True, command=['python', './Tools/Scripts/run-bindings-tests'], ) + 0, ) I'm Ok with the existing style, but it doesn't match what we do outside of buildbot code, and if we want to change things, now is the time. > Tools/CISupport/build-webkit-org/steps_unittest.py:203 > + + 0, I never did understand why we do this "+#" thing after our expect shell commands. > Tools/CISupport/build-webkit-org/steps_unittest.py:220 > + return self.runStep() What motivates returning the result of runStep? That usually is not how Python unit tests work. (In reply to Jonathan Bedard from comment #6) > I'm Ok with the existing style, but it doesn't match what we do outside of buildbot code, and if we want to change things, now is the time. Updated patch uses suggested style. > > Tools/CISupport/build-webkit-org/steps_unittest.py:220 > > + return self.runStep() > > What motivates returning the result of runStep? That usually is not how Python unit tests work. This is copied from existing ews-build unit-tests. Note that we aren't using python inbuilt unittest module, we are using "from twisted.trial import unittest". Created attachment 415169 [details]
Patch
Comment on attachment 415169 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415169&action=review > Tools/CISupport/build-webkit-org/htdigestparser_unittest.py:39 > + digest_file.seek(0) This writes the content at the beginning of the file instead of the end (which was it's previous behavior), is that what we want? (In reply to Jonathan Bedard from comment #9) > This writes the content at the beginning of the file instead of the end (which was it's previous behavior), is that what we want? It doesn't seems to matter for these unit-tests. They are passing for both python 2 and 3 after this change. Committed r270354: <https://trac.webkit.org/changeset/270354> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415169 [details]. |