make isPromise() return boolean value for null and undefined cases

[Jayasankar-m]

• By placing an X in the preceding checkbox, I verify that I have signed the Contributor License Agreement

• Currently isPromise() returns non boolean for the cases like following
  isPromise(null) // returns null
  isPromise(undefined) // returns undefined

• Modified to return boolean value explicitly for the cases mentioned above

---

This change is 

[martin770]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Jayasankar-m)

---

javascript/node/selenium-webdriver/lib/promise.js, line 38 at r1 (raw file):

    // Use array notation so the Closure compiler does not obfuscate away our
    // contract.
    return value != null

!!value to coerce the value to a boolean (instead of value != null)

Also, you should update the tests since they currently do nothing...

/test/lib/promise_test.js -- remove lines 59&60, replace with this test:

it('isPromise', () => {
const v = () => {};
const x = new Promise(v, v);
const p = createRejectedPromise('reject');
assert.equal(true, promise.isPromise(x));
assert.equal(true, promise.isPromise(p));
assert.equal(false, promise.isPromise(0));
assert.equal(false, promise.isPromise(false));
assert.equal(false, promise.isPromise(true));
assert.equal(false, promise.isPromise(null));
assert.equal(false, promise.isPromise(undefined));
assert.equal(false, promise.isPromise(''));
assert.equal(false, promise.isPromise('promise'));
assert.equal(false, promise.isPromise(v));
});

[martin770]

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Jayasankar-m)

---

javascript/node/selenium-webdriver/lib/promise.js, line 38 at r1 (raw file):

Previously, martin770 (James Martin) wrote…

!!value to coerce the value to a boolean (instead of value != null)

Also, you should update the tests since they currently do nothing...

/test/lib/promise_test.js -- remove lines 59&60, replace with this test:

it('isPromise', () => {
const v = () => {};
const x = new Promise(v, v);
const p = createRejectedPromise('reject');
assert.equal(true, promise.isPromise(x));
assert.equal(true, promise.isPromise(p));
assert.equal(false, promise.isPromise(0));
assert.equal(false, promise.isPromise(false));
assert.equal(false, promise.isPromise(true));
assert.equal(false, promise.isPromise(null));
assert.equal(false, promise.isPromise(undefined));
assert.equal(false, promise.isPromise(''));
assert.equal(false, promise.isPromise('promise'));
assert.equal(false, promise.isPromise(v));
});

In fact, you can completely remove the value && part and just have:

        return (typeof value === 'object' || typeof value === 'function')
        && typeof value['then'] === 'function';

And it will still pass all of those tests.

[Jayasankar-m]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @martin770)

---

javascript/node/selenium-webdriver/lib/promise.js, line 38 at r1 (raw file):

Previously, martin770 (James Martin) wrote…

In fact, you can completely remove the value && part and just have:

        return (typeof value === 'object' || typeof value === 'function')
        && typeof value['then'] === 'function';

And it will still pass all of those tests.

Done.
Updated as per review comments

[martin770]

Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved

[CLAassistant]

All committers have signed the CLA.

[diemol]

@Jayasankar-m, would you like to continue with this PR?

[Jayasankar-m]

@Jayasankar-m, would you like to continue with this PR?

@diemol I guess all the required changes are already there in PR. Just needs to merge.
Let me know if anything is missing

[diemol]

@Jayasankar-m, there are merge conflicts, could you please have a look?

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[Jayasankar-m]

@Jayasankar-m, there are merge conflicts, could you please have a look?

@diemol it seems fix to the issue was made by another commit.
The tests written for this are included in the PR

[diemol]

You are right, a recent PR merge fixed this. I will merge the test, thank you.