Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Write tests for start up defense static analysis #15707

Closed
mcomella opened this issue Oct 5, 2020 · 4 comments
Closed

Write tests for start up defense static analysis #15707

mcomella opened this issue Oct 5, 2020 · 4 comments
Assignees
Labels
eng:qa:not-needed Added by QA to issues that cannot be tested performance Possible performance wins

Comments

@mcomella
Copy link
Contributor

mcomella commented Oct 5, 2020

afaik, we're adding three static analysis checks without tests:

We should write tests for them so they don't accidentally regress, considering how important they are to our start up defense strategy.

┆Issue is synchronized with this Jira Task

@mcomella mcomella added the performance Possible performance wins label Oct 5, 2020
@mcomella mcomella added this to Needs prioritization in Performance, front-end roadmap via automation Oct 5, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Oct 5, 2020
@MarcLeclair MarcLeclair moved this from Needs prioritization to Backlog (prioritized) in Performance, front-end roadmap Oct 7, 2020
@liuche liuche removed the needs:triage Issue needs triage label Oct 20, 2020
@mcomella mcomella self-assigned this Oct 21, 2020
@mcomella mcomella moved this from Backlog (prioritized) to In progress in Performance, front-end roadmap Oct 21, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
…out lint.

It caused a test to fail because super.visitElement asserted false
because it expected to be overriden. It was correctly overridden - we
were just still calling through to super anyway.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
I had to ignore one of the tests because the linter test API seemed to
be broken. All of these APIs are beta so I didn't think it was worth
trying to force it to work.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
Apparently these tests haven't been running because they don't compile.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
…t-rules.

Without this, the tests would not run from the command line though they
could run in Android Studio.
@mcomella
Copy link
Contributor Author

I'm adding code to make the tests run in CI (they haven't been) and opened a PR to test that the CI changes work: #16098 (by failing the tests).

mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
For mozilla-detekt-rules, the tests didn't compile at all so apparently
they haven't been running in testing.

mozilla-lint-rules worked but they were not clean.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
…out lint.

It caused a test to fail because super.visitElement asserted false
because it expected to be overriden. It was correctly overridden - we
were just still calling through to super anyway.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
I had to ignore one of the tests because the linter test API seemed to
be broken. All of these APIs are beta so I didn't think it was worth
trying to force it to work.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
…t-rules.

Without this, the tests would not run from the command line though they
could run in Android Studio.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
This works but it's imperfect: when the tests fail, it links to the lint
failure artifacts rather than the test failure artifacts.
@mcomella
Copy link
Contributor Author

Opened a PR: #16101

mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
For mozilla-detekt-rules, the tests didn't compile at all so apparently
they haven't been running in testing.

mozilla-lint-rules worked but they were not clean.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
…out lint.

It caused a test to fail because super.visitElement asserted false
because it expected to be overriden. It was correctly overridden - we
were just still calling through to super anyway.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
I had to ignore one of the tests because the linter test API seemed to
be broken. All of these APIs are beta so I didn't think it was worth
trying to force it to work.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 21, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 28, 2020
…out lint.

It caused a test to fail because super.visitElement asserted false
because it expected to be overriden. It was correctly overridden - we
were just still calling through to super anyway.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 28, 2020
I had to ignore one of the tests because the linter test API seemed to
be broken. All of these APIs are beta so I didn't think it was worth
trying to force it to work.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 28, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 28, 2020
…t-rules.

Without this, the tests would not run from the command line though they
could run in Android Studio.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 28, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 28, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 28, 2020
This works but it's imperfect: when the tests fail, it links to the lint
failure artifacts rather than the test failure artifacts.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 28, 2020
…d-gradle-dependencies script.

`assemble` doesn't assemble the tests so we need to run `test`.
`testClasses` isn't good enough because, according to `--profile`, it
doesn't include dependencies for `testRuntimeClasspath`.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
For mozilla-detekt-rules, the tests didn't compile at all so apparently
they haven't been running in testing.

mozilla-lint-rules worked but they were not clean.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
…out lint.

It caused a test to fail because super.visitElement asserted false
because it expected to be overriden. It was correctly overridden - we
were just still calling through to super anyway.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
I had to ignore one of the tests because the linter test API seemed to
be broken. All of these APIs are beta so I didn't think it was worth
trying to force it to work.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
…t-rules.

Without this, the tests would not run from the command line though they
could run in Android Studio.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
This works but it's imperfect: when the tests fail, it links to the lint
failure artifacts rather than the test failure artifacts.
mcomella added a commit to mcomella/fenix that referenced this issue Oct 31, 2020
…d-gradle-dependencies script.

`assemble` doesn't assemble the tests so we need to run `test`.
`testClasses` isn't good enough because, according to `--profile`, it
doesn't include dependencies for `testRuntimeClasspath`.
mcomella added a commit that referenced this issue Oct 31, 2020
For mozilla-detekt-rules, the tests didn't compile at all so apparently
they haven't been running in testing.

mozilla-lint-rules worked but they were not clean.
mcomella added a commit that referenced this issue Oct 31, 2020
It caused a test to fail because super.visitElement asserted false
because it expected to be overriden. It was correctly overridden - we
were just still calling through to super anyway.
mcomella added a commit that referenced this issue Oct 31, 2020
I had to ignore one of the tests because the linter test API seemed to
be broken. All of these APIs are beta so I didn't think it was worth
trying to force it to work.
mcomella added a commit that referenced this issue Oct 31, 2020
Without this, the tests would not run from the command line though they
could run in Android Studio.
mcomella added a commit that referenced this issue Oct 31, 2020
This works but it's imperfect: when the tests fail, it links to the lint
failure artifacts rather than the test failure artifacts.
mcomella added a commit that referenced this issue Oct 31, 2020
…dencies script.

`assemble` doesn't assemble the tests so we need to run `test`.
`testClasses` isn't good enough because, according to `--profile`, it
doesn't include dependencies for `testRuntimeClasspath`.
@mcomella
Copy link
Contributor Author

Addressed in #16101

Performance, front-end roadmap automation moved this from In progress to Done Oct 31, 2020
@mcomella mcomella added the eng:qa:not-needed Added by QA to issues that cannot be tested label Oct 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:qa:not-needed Added by QA to issues that cannot be tested performance Possible performance wins
Development

No branches or pull requests

2 participants