1654396 - Run `mach lint` (or some suitable test there-of) on Windows in automation

Status: NEW

(Developer Infrastructure :: Lint and Formatting, task)

Description

[Nick Alexander :nalexander [he/him]]

This ticket tracks running mach lint, or some test suite that ensures that it can be run, on Windows in automation.

Mozilla actively wants developers to work on Windows. mach lint is an essential part of the developer work flow; it has to work on Windows. I have personally witnessed two failures with mach lint that are Windows specific. One of them -- Bug 1648586 -- was 100% deterministic and was broken for every single invocation of mach lint on Windows. I think Bug 1654395 is probably the same.

We need to guard against disruptions to the essential parts of the developer work flow (on all platforms), but especially on Windows, because the majority of developers are not (yet) using that platform.

Comment 1

[Nick Alexander :nalexander [he/him]]

A good first cut for this would be to run one existing lint job on Windows as a smoke test.

Comment 2

[Andrew Halberstadt [:ahal]]

Both the mozlint unit tests and the linter integration tests already do run on Windows. Though admittedly some of the integration tests are skipped due to missing dependencies.

Our test coverage is certainly not 100%.. However in my experience, I am very rarely able to reproduce Windows issues like bug 1654395 that are caused by different encodings (and the existing CI doesn't catch them either). It's not 100% clear to me how we should be testing for this (my knowledge is lacking in this area).

Bug 1648586 was an error in a specific linter (pylint) whose test happens to be disabled on Windows. We should absolutely spend some effort getting its test (and the others) working on Windows. (As an aside, I don't consider myself responsible for our lint integrations. I maintain the core infrastructure as best I can, but otherwise ownership in linting is lacking).

Anyway, all that to say that:

1. Yes, I agree there are a lot of Windows issues that pop and we should do better.
2. I'd be tempted to resolve this bug in favour of bugs that track enabling those last remaining lint integrations on Windows.
3. Not sure what to do about non utf-8 encoding bugs.. I'm sure they're typically legit, but I rarely know how to fix them.

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Andrew Halberstadt [:ahal] from comment #2)

Both the mozlint unit tests and the linter integration tests already do run on Windows. Though admittedly some of the integration tests are skipped due to missing dependencies.

Huzzah! Then this bug can either be closed or morphed into "actually run some lints on Windows". If I understand correctly we don't do the latter -- can you confirm?

Our test coverage is certainly not 100%.. However in my experience, I am very rarely able to reproduce Windows issues like bug 1654395 that are caused by different encodings (and the existing CI doesn't catch them either). It's not 100% clear to me how we should be testing for this (my knowledge is lacking in this area).

I agree, this particular ticket isn't representative. Encodings are terrible and almost impossible to test thoroughly.

Bug 1648586 was an error in a specific linter (pylint) whose test happens to be disabled on Windows. We should absolutely spend some effort getting its test (and the others) working on Windows. (As an aside, I don't consider myself responsible for our lint integrations. I maintain the core infrastructure as best I can, but otherwise ownership in linting is lacking).

That's totally fair -- you're the first point of contact, not responsible.

Anyway, all that to say that:

1. Yes, I agree there are a lot of Windows issues that pop and we should do better.

s/... for lint/... for any part of Mozilla/ :)

2. I'd be tempted to resolve this bug in favour of bugs that track enabling those last remaining lint integrations on Windows.

I'd be fine with that, but there still seems to be utility in actually running a small set of lints -- not just the tests -- on Windows. Am I not reading the situation correctly?

3. Not sure what to do about non utf-8 encoding bugs.. I'm sure they're typically legit, but I rarely know how to fix them.

I think it's fine for motivated parties (myself under Windows, glandium for Japanese locales, etc) to address them as and when.

Comment 4

[Andrew Halberstadt [:ahal]]

(In reply to Nick Alexander :nalexander [he/him] from comment #3)

(In reply to Andrew Halberstadt [:ahal] from comment #2)

Both the mozlint unit tests and the linter integration tests already do run on Windows. Though admittedly some of the integration tests are skipped due to missing dependencies.

Huzzah! Then this bug can either be closed or morphed into "actually run some lints on Windows". If I understand correctly we don't do the latter -- can you confirm?

Correct, they only run on Linux (since that is cheapest and the actual issues that get generated should theoretically be platform independent).

there still seems to be utility in actually running a small set of lints -- not just the tests -- on Windows. Am I not reading the situation correctly?

You're probably right, maybe we should just run all our lints on Windows? Though we'd need to be careful about cost (both dollars and time), as things like reviewbot want the results ASAP.

I guess my thinking was that if we actually had our linter integration tests fully enabled (and with decent code coverage), then running them on Windows wouldn't provide any additional value. But that is not the current reality, so it might be a good measure to implement in the meantime.
Then again, if we try to run the actual lint tasks themselves on Windows we'll run into the same bootstrapping problems that the tests have. So at that point maybe it's best to just fix the tests and leave the real lints alone (given Linux is faster and cheaper).

Comment 5

[Andrew Halberstadt [:ahal]]

One immediate change I will personally make is being more strict with new linter reviews (requiring tests on all platforms before it can land).

Comment 6

[Andrew Halberstadt [:ahal]]

Also for historical context, a lot of those linters initially landed without tests (since we were a lot less strict with reviews back in the day). So the tests got added after the fact and were Linux only since it is trivial to add dependencies to the docker image there. Whereas Windows / Mac are a pain to update.