-
Notifications
You must be signed in to change notification settings - Fork 216
Feature/java integration tests #300
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
Conversation
…es-operator into feature/java-integration-tests
integration-tests/.gitignore
Outdated
@@ -0,0 +1 @@ | |||
/target/ |
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.
There is a top-level .gitignore file, so this should be redundant
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.
deleted this file
integration-tests/pom.xml
Outdated
@@ -0,0 +1,212 @@ | |||
<!-- Copyright 2017, Oracle Corporation. All Rights Reserved. --> |
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.
License template for all files (okay to use XML comment style here)
// Copyright 2018, Oracle Corporation and/or its affiliates. All rights reserved.
// Licensed under the Universal Permissive License v 1.0 as shown at
// http://oss.oracle.com/licenses/upl.
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.
added license in all files
integration-tests/pom.xml
Outdated
@@ -0,0 +1,212 @@ | |||
<!-- Copyright 2017, Oracle Corporation. All Rights Reserved. --> | |||
<!-- This is unreleased proprietary source code of Oracle Corporation --> |
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.
Comment needs to be removed.
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.
done
integration-tests/pom.xml
Outdated
<properties> | ||
<src-integration-test>${project.basedir}/src/integration-tests/java</src-integration-test> | ||
<resource-integration-test>${project.basedir}/src/integration-tests/resources</resource-integration-test> | ||
<docker-image-name>container-registry.oracle.com/middleware/weblogic-kubernetes-operator:latest</docker-image-name> |
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.
I don't understand this. Why would we only test against the latest from OCR? Shouldn't we test against what we build?
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.
I see that below you build to this image name. It's still kind of confusing though.
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.
I changed the image name based on the git branch name, moved docker build to sh script as the image name is dynamic. please see the latest pom.xml and pullimages.sh is renamed to setupenv.sh.
@@ -0,0 +1,201 @@ | |||
package oracle.kubernetes.operator; |
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.
All Java files need the license
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.
done
|
||
//@Test | ||
public void testElkIntegration() { | ||
//TODO in run.sh |
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.
Please either implement or remove.
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.
removed
String outputStr = TestUtils.executeCommand(cmd.toString()); | ||
logger.info("run " + outputStr); | ||
|
||
//TODo: Add check for the image name from the Pod as in run.sh |
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.
Please implement or remove
//TODo: Add check for the image name from the Pod as in run.sh | ||
|
||
if (!outputStr.contains(createScriptMessage)) { | ||
//use logger |
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.
Please implement using logger or remove.
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.
done
BufferedReader reader = new BufferedReader(new InputStreamReader(p.getInputStream())); | ||
BufferedReader errReader = new BufferedReader(new InputStreamReader(p.getErrorStream())); | ||
|
||
//in some cases u may want to read process error stream as well |
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.
Explain?
@@ -0,0 +1,8 @@ | |||
docker pull wlsldi-v2.docker.oraclecorp.com/store-weblogic-12.2.1.3:latest |
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.
We can't expose these internal addresses
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.
this is how its done in run.sh as well
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.
please let me know if there other ways to do it
File f = new File(BaseTest.class.getClassLoader().getResource(appPropsFile).getFile()); | ||
if (!f.exists()) { | ||
throw new IllegalArgumentException( | ||
"FAILURE: Invalid operator appp properties file " + appPropsFile); |
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.
Can you re-use any of the operator test utils dealing with inputs files and output directories?
e.g. operator/src/test/java/oracle/kubernetes/operator/utils/UserProjects.java ?
Or are you not able to use them because you're under a different root directory (integration-tests/src/...)?
protected static Properties opProps = new Properties(); | ||
protected static Properties domainProps = new Properties(); | ||
protected static Properties appProps = new Properties(); | ||
|
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.
Please add comments explaining what the protected static variables are used for. e.g. I'm not sure what the op/domain/app props files are used for. e.g. are opProps and domainProps the inputs files to the create operator / domain shell scripts?
* | ||
* @author Vanajakshi Mukkara | ||
*/ | ||
public class BaseTest { |
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.
Why are all these properties (e.g. operator, ...) protected static with a setup method instead of member data with a constructor?
Why are they protected (v.s. private with getters and setters?)
protected static String domainUid = ""; | ||
protected static String domainNS = ""; | ||
|
||
public static void setup() throws Exception { |
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.
This is a very long function. How about breaking it up into a bunch of smaller ones? Might help with re-use and it will make the code clearer.
*/ | ||
@BeforeClass | ||
public static void staticPrepare() throws Exception { | ||
logger.info("+++++++++++++++++++++++++++++++++---------------------------------+"); |
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.
You're using this pattern of logger.info +++++ / begin / message in several places. How about writing a function that does it?
logger.info("BEGIN"); | ||
logger.info("Run once, Creating Operator & " + "waiting for the script to complete execution"); | ||
|
||
opPropsFile = "ITSingleDomain_op.properties"; |
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.
Why aren't these args to setup? i.e. it seems odd to set static variables then call a setup function that uses them to do other stuff.
StringBuffer cmd = new StringBuffer(); | ||
cmd.append("kubectl get pod ").append(podName).append(" -n ").append(domainNS); | ||
|
||
while (i < MAX_ITERATIONS_POD) { |
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.
How about adding a utility that does the looping where you can pass in the code that does the check?
v.s. repeating the while loop, sleep, max iterations stuff a bunch of places.
.append(clusterName) | ||
.append("\")].replicas }'"); | ||
logger.fine("getClusterReplicas cmd =" + cmd); | ||
String output = TestUtils.executeCommand(new String[] {"/bin/sh", "-c", cmd.toString()}); |
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.
I think I've seen the /bin/sh -c stuff a bunch of places - how about adding another variant of executeCommand that does this?
} | ||
|
||
private static int makeOperatorRestCall( | ||
String operatorNS, String url, String jsonObjStr, String userProjectsDir) throws Exception { |
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.
Long function - split it up maybe?
TestUtils.executeCommand(new String[] {"/bin/sh", "-c", opKeyDecodeCmd.toString()}); | ||
return keyFile.getAbsolutePath(); | ||
} | ||
|
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.
what does cleanupAll do and when should you call it?
adminServerName= admin-server | ||
domainName= base_domain | ||
domainUID= domain1 | ||
startupControl= AUTO |
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.
Remove default values? e.g. startupControl = AUTO
(this will make it easier to migrate your tests to the new lifecycle and WDT stuff)
@vanajamukkara I've figured out why the format validation isn't running on your files. You need to conform to standard Maven directory layouts. Please "git mv" integration-tests/src/integration-tests to integration-tests/src/test. |
…es-operator into feature/java-integration-tests
…racle/weblogic-kubernetes-operator into feature/java-integration-tests
…es-operator into feature/java-integration-tests
This is for the issue #299
Currently these tests cover QUICK_TEST use cases in run.sh for "Standalone" mode.
To run the tests,
mvn clean install -P java-integration-tests
or
mvn test-compile integration-test -P java-integration-tests
http://aseng-wiki.****/asengwiki/display/ASQA/Weblogic+Operator+Integration+Tests+in+Java