Issue #17910: Remove suppressions for the ImportControlTest rule#17911
Conversation
cbe1333 to
25b6495
Compare
|
@mureinik , thanks a lot for huge update. Impressed. Please rebase to latest, as I just merged PR what used reflection to cover close-to-impossible situation by test. Please squash all to single commit. FYI: |
25b6495 to
97028d7
Compare
|
There are some CI failures around mutation coverage, looking in to it. |
83d9366 to
12ea33b
Compare
…h configuration in import-control-test.xml
12ea33b to
21141a0
Compare
|
@romani :
done.
and done. with the latest revision of this PR, all CI checks pass. |
romani
left a comment
There was a problem hiding this comment.
Thanks a lot.
This code part did get much love from community for long time.
I appreciate your time , I know that passing our CI is not easy
|
I appreciate your unexpected appearance in project please let me know how we how we can extend our collaboration. |
Honestly, I just happened to have some downtime with the local public holidays, and wanted to see if I could help a project I've been using for years. |
The
ImprotControlTestrule should govern which imports aren't (and are) allowed in test files. However, for many tests, this rule is suppressed, so the configuration is not adhered to, leading to classes that aren't supposed to be used in tests.This PR aims to address this issue.
It removes these suppressions, and where needed, replaces them with finer-grained "allow" entries in
import-control-test.xml.This PR addresses the following concerns:
Usage of reflection
TestUtilthat provide this functionality, so there's a clear (or at least, clearer) separation of responsibilities.<allow>entries inimport-control-test.xml. There may be room for additional improvement here in the future, but it is out of scope for this PR.java.lang.reflectparameters or return values were explicitly allowed to import those classes with<allow>entries.Usage of JUnit
assertThrowsassertions were replaced withTestUtil. getExpectedThrowable, as per WrapassertThrowswithgetExpectedThrowablemethod. #11343assertDoesNotThrowwere written with Truth assertions instead of JUnit assertions.import-control-test.xmlsyntax for disallowing JUnit and Hamcrest assertions was fixed.Fixes #17910