1666456 - Tests for assert.positiveNumber are missing

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As noticed while working on the spec, our assert.positiveNumber implementation doesn't check for the maximum safe integer value similar to what assert.number does.

https://searchfox.org/mozilla-central/rev/b58ca45005fe02077c92779483d1b60e9a49687c/testing/marionette/assert.js#228-246

We should get this corrected.

Comment 1

[Garima]

hi, Is this bug still open. Can you assign it to me ? I would love to work on it.

Comment 2

[Anshuman Singh]

Hi, is anyone working on it ?I would like to work on this issue.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Garima from comment #1)

hi, Is this bug still open. Can you assign it to me ? I would love to work on it.

Yes, this bug is still open as you can see (not marked as fixed) and as such feel free to take it. Note that bugs are assigned automatically when a patch gets uploaded. In case of questions don't hesitate to ask. Thanks.

Comment 4

[Garima]

Can you please guide me how to start with it. Actually I am new to open source. How can I see the way things are working And how they should actually work on my local? And from where should I start? Even small help will work. I genuinely want to learn.
Thank You

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

A good documentation for getting started with Marionette (WebDriver) development you can find here:
https://firefox-source-docs.mozilla.org/testing/marionette/NewContributors.html

It has links to answers for all of your questions above. But don't hesitate to ask further, as you already did on Element. Thanks!

Comment 6

[Garima]

Ok Thanks

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Garima, I want to check back and ask if everything is clear and that you want to still work on this bug. Please let us know. Thanks.

Comment 8

[Garima]

I am so sorry actually I had exams and therefore I was not able to give time to this. I'll now start looking at it again

Comment 9

[Garima]

Hi, Went through the doc but its still confusing from where to begin. Can I first begin with cloning this code into my local. But cannot even find clone option. Plz help

Comment 10

[Garima]

Or need to setup marionette first

Comment 11

[sayuree]

Attachment: Bug 1666456 - added max safe integer check. r=whimboo

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As discovered while reviewing the patch we already have a finite check for assert.positiveNumber but actually not a single test. So I've updated the bug's summary to reflect that we want to have this test.

Comment 13

[sayuree]

Attachment: Bug 1666456 - [remote] added tests to Assert.positiveNumber. r=whimboo,jdescottes

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/381966465ebe
[remote] added tests to Assert.positiveNumber. r=whimboo,jdescottes,webdriver-reviewers

Comment 15

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/381966465ebe