1452238 - test_alerts_post doesn't set or test the PerformanceDatum's job_id

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect, P1)

Description

[Ed Morley [:emorley]]

eslint reported:
tests/webapp/api/test_performance_alerts_api.py:174,18: Unused variable 'job_id' (W0612: unused-variable)

Which refers to:
https://github.com/mozilla/treeherder/blob/7beb3cacc3c846eff2e6524be67a05a265772015/tests/webapp/api/test_performance_alerts_api.py#L174

It looks like `job_id` should be being used, and be passed to PerformanceDatum when creating.

Or else if not, it should be removed from the zip().

Comment 1

[Ionuț Goldan [:igoldan]]

The problem is still there, but that piece of code has been moved here.

Comment 2

[Ganesh Rajasekar]

Hello,

I would like to work on this issue. This is my very first time contributing it would be great if you could redirect me to the documentation and kindly please guide me on how to get started on this. Thank you

Comment 3

[Ionuț Goldan [:igoldan]]

Ganesh, are you still interested in working on this bug? You can find the Treeherder docs here.

Comment 4

[Ganesh Rajasekar]

Thanks for your reply. Sure I would love to work on ths bug. I have looked at how to fix the issue. I will look at the docs and raise a PR for the same. This would be my first PR ever please do let me know if I should take care of anything

Comment 5

[Suyash Salampuria]

I think that passing ds_job_id as job_id would be enough .
Shall I go on proceeding this way

Comment 6

[Ionuț Goldan [:igoldan]]

(In reply to suyash546 from comment #5)

I think that passing ds_job_id as job_id would be enough .
Shall I go on proceeding this way

I don't think that's the best approach. ds_job_id is a deprecated field & the tests should take that into account.

Comment 7

[Ionuț Goldan [:igoldan]]

From what I can see, job ids are nullable, which means they don't have to be provided.
Thus, we should simply remove the mentioning of that job_id from the zip entirely.

Comment 8

[Suyash Salampuria]

Shall I go on to do this

Comment 9

[Ionuț Goldan [:igoldan]]

(In reply to suyash546 from comment #8)

Shall I go on to do this

Please do.

Comment 10

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/5956

Comment 11

[Armen [:armenzg]]

Well done Suyash!

Comment 12

[Suyash Salampuria]

Thanks Armen!!