New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Low level Rest Client #18735
Low level Rest Client #18735
Changes from 1 commit
3efbe95
c9c33a7
5970723
6ae8be5
2589235
94cf843
f6a5a0a
ce663e9
a472544
062a216
bd29dc1
9ffdea9
d7c4176
e85ed3e
e77ab87
530ad22
e7fe397
17a21f0
e040d2f
9569ebc
599dad5
b38ef34
cdffc3d
85a7721
c0a72c1
2d7a781
2735897
3890842
1d06916
16ab491
9a38d81
6d3f6c7
325b723
eae914a
c70e08c
e81aad9
6490355
29b1f8d
7f4807b
044a97c
3745305
51e487f
83c6e73
c9db111
6d66fbd
35dbdee
47e5204
24ea585
9ed2d61
4572b69
13a27a3
4fa824f
f5825b8
23a94bb
c48b7a7
29eee32
f17f0f9
b891c46
b15279b
f2318ed
56e689e
05e26a4
a461dd8
467dfbd
1f7f6e2
2cf04c0
791db1f
a78fe13
04d620d
be5e2e1
437c4f2
85606e8
c028bf9
742f9c6
853ea93
cbdffc7
de8b9fd
9a4971d
d8c0fad
3474a14
33bdab1
bd77335
cf6e713
f38ce72
2856ab0
0af9d2c
a5e329e
9cbfa98
432efc7
6db90da
3d7186c
656422c
777d438
50b6f4c
3cd201e
8f7b7fb
116805b
caa6c96
1932f6b
cf93e90
70fb67c
7c8013f
ace3a7b
8c60374
af93533
cb4bfcb
886cb37
f0b6abe
490d9c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,31 +44,36 @@ | |
*/ | ||
public class NamingConventionsCheck { | ||
public static void main(String[] args) throws IOException { | ||
int i = 0; | ||
NamingConventionsCheck check = new NamingConventionsCheck( | ||
loadClassWithoutInitializing(args[i++]), | ||
loadClassWithoutInitializing(args[i++])); | ||
Class<?> testClass = null; | ||
Class<?> integTestClass = null; | ||
Path rootPath = null; | ||
boolean skipIntegTestsInDisguise = false; | ||
boolean selfTest = false; | ||
while (true) { | ||
switch (args[i]) { | ||
case "--skip-integ-tests-in-disguise": | ||
skipIntegTestsInDisguise = true; | ||
i++; | ||
continue; | ||
case "--self-test": | ||
selfTest = true; | ||
i++; | ||
continue; | ||
case "--": | ||
i++; | ||
break; | ||
default: | ||
fail("Expected -- before a path."); | ||
for (int i = 0; i < args.length; i++) { | ||
String arg = args[i]; | ||
switch (arg) { | ||
case "--test-class": | ||
testClass = loadClassWithoutInitializing(args[++i]); | ||
break; | ||
case "--integ-test-class": | ||
integTestClass = loadClassWithoutInitializing(args[++i]); | ||
break; | ||
case "--skip-integ-tests-in-disguise": | ||
skipIntegTestsInDisguise = true; | ||
break; | ||
case "--self-test": | ||
selfTest = true; | ||
break; | ||
case "--": | ||
rootPath = Paths.get(args[++i]); | ||
break; | ||
default: | ||
fail("unsupported argument '" + arg + "'"); | ||
} | ||
break; | ||
} | ||
check.check(Paths.get(args[i])); | ||
|
||
NamingConventionsCheck check = new NamingConventionsCheck(testClass, integTestClass); | ||
check.check(rootPath, skipIntegTestsInDisguise); | ||
|
||
if (selfTest) { | ||
assertViolation("WrongName", check.missingSuffix); | ||
|
@@ -87,9 +92,9 @@ public static void main(String[] args) throws IOException { | |
assertNoViolations("Found inner classes that are tests, which are excluded from the test runner", check.innerClasses); | ||
assertNoViolations("Pure Unit-Test found must subclass [" + check.testClass.getSimpleName() + "]", check.pureUnitTest); | ||
assertNoViolations("Classes ending with [Tests] must subclass [" + check.testClass.getSimpleName() + "]", check.notImplementing); | ||
if (!skipIntegTestsInDisguise) { | ||
assertNoViolations("Subclasses of ESIntegTestCase should end with IT as they are integration tests", | ||
check.integTestsInDisguise); | ||
if (skipIntegTestsInDisguise == false) { | ||
assertNoViolations("Subclasses of " + check.integTestClass.getSimpleName() + | ||
" should end with IT as they are integration tests", check.integTestsInDisguise); | ||
} | ||
} | ||
|
||
|
@@ -108,7 +113,7 @@ public NamingConventionsCheck(Class<?> testClass, Class<?> integTestClass) { | |
this.integTestClass = integTestClass; | ||
} | ||
|
||
public void check(Path rootPath) throws IOException { | ||
public void check(Path rootPath, boolean skipTestsInDisguised) throws IOException { | ||
Files.walkFileTree(rootPath, new FileVisitor<Path>() { | ||
/** | ||
* The package name of the directory we are currently visiting. Kept as a string rather than something fancy because we load | ||
|
@@ -143,7 +148,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO | |
String className = filename.substring(0, filename.length() - ".class".length()); | ||
Class<?> clazz = loadClassWithoutInitializing(packageName + className); | ||
if (clazz.getName().endsWith("Tests")) { | ||
if (integTestClass.isAssignableFrom(clazz)) { | ||
if (skipTestsInDisguised == false && integTestClass.isAssignableFrom(clazz)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd just check if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, I disagree, I didn't add a new boolean but just made work the existing one, no need to load the class if those checks are disabled. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok with merging as is and hacking on it later. I don't like that the boolean which means "I don't care if integ tests sneak into the regular test phase" also means "there aren't integ tests so don't bother looking" but there are worse problems in the world. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure... I see it differently though. we simply don't need to load the class if we don't do any check. We maybe could simply rename the option to "skipIntegTestCheck" or something. let's postpone this though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ++ |
||
integTestsInDisguise.add(clazz); | ||
} | ||
if (Modifier.isAbstract(clazz.getModifiers()) || Modifier.isInterface(clazz.getModifiers())) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,8 +61,11 @@ forbiddenApisTest { | |
|
||
//JarHell is part of es core, which we don't want to pull in | ||
jarHell.enabled=false | ||
//NamingConventionCheck is part of test-framework, which we don't want to pull in as it depends on es core | ||
namingConventions.enabled=false | ||
|
||
namingConventions { | ||
//we don't have integration tests | ||
skipIntegTestInDisguise = true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer settings integTestClass to null or defaulting it to null and setting it our stuff somewhere else in the build. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tried defaulting it to null, but ended up having to set it in too many projects. Seems like we should rather disable in the client exception only? I went for setting it to null here explicitly but those null values become trickier to handle and understand, I went for the boolean that we already had cause it's more explicit and considered it a bug that when the check is disabled we still load the class. |
||
} | ||
|
||
dependencyLicenses { | ||
dependencies = project.configurations.runtime.fileCollection { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add the
integTestClass != null
here too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it can't happen to have it null when skipIntegTestsInDisguise is false no?