1479513 - [meta] Fix instances of uses of undefined constants in IDL files

Status: RESOLVED FIXED

(Toolkit :: General, task, P3)

Description

[Mark Banner (:standard8)]

It was noticed in a couple of bugs that we were attempting to access constants in IDL files that are undefined.

I whipped up a rule using ESLint (which can't land in its current state), that found a bunch of places where we're trying to access incorrect items.

This bug is to track the fixing of those issues.

Comment 1

[:Gijs (he/him)]

(In reply to Mark Banner (:standard8) from comment #0)
> It was noticed in a couple of bugs that we were attempting to access
> constants in IDL files that are undefined.
> 
> I whipped up a rule using ESLint (which can't land in its current state),
> that found a bunch of places where we're trying to access incorrect items.
> 
> This bug is to track the fixing of those issues.

Can you put up the script somewhere? Do you have a plan of how to integrate it into CI so we don't reintroduce issues like these?

Comment 2

[Mark Banner (:standard8)]

Attachment: Not commitable IDL constants checker

(In reply to :Gijs (he/him) from comment #1)
> Can you put up the script somewhere?

Attached. This runs as part of ESLint, but relies on a build to have been created first, so that we have the xpt information. It also assumes my object directory path.

I'm also not sure it'd work in an artifact build...

> Do you have a plan of how to integrate it into CI so we don't reintroduce issues like these?

Not really. I don't think it is really ESLint's concern to actually test this, so I think it would need a different analyser of some form. ESLint was just a useful tool that I could whip up a prototype that gave us answers.

Currently I'm just running it occasionally to check for any new issues.

Comment 3

[:Gijs (he/him)]

Through some recent new performance tests, I found out about cron.yml. Could we use that to run a job that checks this once a day, as tier-2, so that the job of auditing this doesn't fall on you, and instead we can back out patches that add new instances? :-)

Comment 4

[Mark Banner (:standard8)]

(In reply to :Gijs (he/him) from comment #3)

Through some recent new performance tests, I found out about cron.yml. Could we use that to run a job that checks this once a day, as tier-2, so that the job of auditing this doesn't fall on you, and instead we can back out patches that add new instances? :-)

Yes, we should look at doing that, cron seems the right way. I've just filed bug 1666491 for the implementation.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 9002518 [details] [diff] [review]
Not commitable IDL constants checker

Moving patches to bug 1479515.

Comment 6

[Mark Banner (:standard8)]

Marking as fixed - we now have a tier-2 builder that is watching for new instances (bug 1666491, bug 1800874) and they will be filed accordingly.