Bug 171362 - Enhance shouldBe()/shouldNotBe() to accept anonymous function arguments
Summary: Enhance shouldBe()/shouldNotBe() to accept anonymous function arguments
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks: 171424
  Show dependency treegraph
 
Reported: 2017-04-26 19:18 PDT by David Kilzer (:ddkilzer)
Modified: 2017-04-29 01:48 PDT (History)
14 users (show)

See Also:


Attachments
Patch v1 (26.41 KB, patch)
2017-04-27 11:31 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch v2 (31.25 KB, patch)
2017-04-27 12:55 PDT, David Kilzer (:ddkilzer)
joepeck: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2017-04-26 19:18:54 PDT
We can enhance shouldBe()/shouldNotBe() from js-test-pre.js and js-test.js to take anonymous function arguments (in addition to string arguments) to make it easier to capture local variables inside expressions.

See also Bug 159232 for a similar change to shouldThrow()/shouldNotThrow().
Comment 1 Radar WebKit Bug Importer 2017-04-27 11:08:16 PDT
<rdar://problem/31867686>
Comment 2 David Kilzer (:ddkilzer) 2017-04-27 11:31:11 PDT
Created attachment 308416 [details]
Patch v1
Comment 3 David Kilzer (:ddkilzer) 2017-04-27 12:55:44 PDT
Created attachment 308432 [details]
Patch v2
Comment 4 Joseph Pecoraro 2017-04-27 13:52:39 PDT
Comment on attachment 308432 [details]
Patch v2

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

r=me

These files seem to vary between 2 and 4 spaces indentation. You can fix if you want, or leave mixed but eliminate the 3s =)

> LayoutTests/ChangeLog:43
> +        to shoudlBe() into an anonymous function.

Typo: "shoudlBe"

> LayoutTests/http/tests/resources/js-test-pre.js:219
> +  try {
> +     _av = eval(_a);
> +  } catch (e) {

Style: indent is 3 instead of 2.

How did this file come to have 2 space indent anyways. Weird.

> LayoutTests/http/tests/resources/js-test-pre.js:647
> +     return _av.then(function(result) {

Style: whitespace is slightly off
Comment 5 David Kilzer (:ddkilzer) 2017-04-27 14:30:09 PDT
(In reply to Joseph Pecoraro from comment #4)
> Comment on attachment 308432 [details]
> Patch v2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308432&action=review
> 
> r=me
> 
> These files seem to vary between 2 and 4 spaces indentation. You can fix if
> you want, or leave mixed but eliminate the 3s =)
> 
> > LayoutTests/ChangeLog:43
> > +        to shoudlBe() into an anonymous function.
> 
> Typo: "shoudlBe"
> 
> > LayoutTests/http/tests/resources/js-test-pre.js:219
> > +  try {
> > +     _av = eval(_a);
> > +  } catch (e) {
> 
> Style: indent is 3 instead of 2.
> 
> How did this file come to have 2 space indent anyways. Weird.
> 
> > LayoutTests/http/tests/resources/js-test-pre.js:647
> > +     return _av.then(function(result) {
> 
> Style: whitespace is slightly off

Thanks!  Will fix whitespace issues in a separate commit.  (I want to test the new checker while making these changes as well.)
Comment 6 David Kilzer (:ddkilzer) 2017-04-27 14:30:26 PDT
Committed r215894: <http://trac.webkit.org/changeset/215894>
Comment 7 David Kilzer (:ddkilzer) 2017-04-29 01:48:16 PDT
(In reply to David Kilzer (:ddkilzer) from comment #5)
> (In reply to Joseph Pecoraro from comment #4)
> > Comment on attachment 308432 [details]
> > Patch v2
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=308432&action=review
> > 
> > r=me
> > 
> > These files seem to vary between 2 and 4 spaces indentation. You can fix if
> > you want, or leave mixed but eliminate the 3s =)
> > 
> > > LayoutTests/ChangeLog:43
> > > +        to shoudlBe() into an anonymous function.
> > 
> > Typo: "shoudlBe"
> > 
> > > LayoutTests/http/tests/resources/js-test-pre.js:219
> > > +  try {
> > > +     _av = eval(_a);
> > > +  } catch (e) {
> > 
> > Style: indent is 3 instead of 2.
> > 
> > How did this file come to have 2 space indent anyways. Weird.
> > 
> > > LayoutTests/http/tests/resources/js-test-pre.js:647
> > > +     return _av.then(function(result) {
> > 
> > Style: whitespace is slightly off
> 
> Thanks!  Will fix whitespace issues in a separate commit.  (I want to test
> the new checker while making these changes as well.)

Bug 171424: check-webkit-style should keep JavaScript test functions in sync