Skip to content
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

1551 run config12 tck #1725

Merged
merged 1 commit into from
Jan 25, 2018
Merged

Conversation

tevans78
Copy link
Member

No description provided.

@tevans78 tevans78 self-assigned this Jan 22, 2018
@tevans78 tevans78 requested a review from hutchig January 22, 2018 18:54
@LibbyBot
Copy link

Code analysis and actions

DO NOT DELETE THIS COMMENT.
  • 67 product code files were changed.

  • Please describe in a separate comment how you tested your changes.

  • 14 infrastructure code files were changed.

  • 2 test infrastructure code files were changed.

  • Test failures/errors in the build could be due to these changes.

  • 70 FAT files were changed, added, or removed.

  • Check that the build did not break the affected FAT suite(s).

@tevans78 tevans78 force-pushed the 1551-runConfig12TCK branch from 2d3f364 to 7472a8e Compare January 22, 2018 18:58
@hutchig
Copy link
Contributor

hutchig commented Jan 22, 2018

Tom you have pulled in two commits from 10 days ago as part of the merge that are part of 1491. That I know Neil has been reviewing here: #1670.
Pulling in un-reviewed commits as part of a merge was something you spoke to me about the last time I got a review from you - but in that case it was other squads commits - in this case it is commits that Neil is in the process go going through. I will review and comment on the TCK related commits and chat to you in the morning. If Neil has not started his review and is happy I can review the 1670 commits too quite happily - but there is a lot code to check twice :-)

@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>
<projectDescription>
<name>com.ibm.ws.microprofile.config.1.2_fat_tck</name>
Copy link
Contributor

@hutchig hutchig Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edited/changed project name (from Eclipse).

*******************************************************************************/

dependencies {
requiredLibs 'org.hamcrest:hamcrest-all:1.3'
Copy link
Contributor

@hutchig hutchig Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maven co-ordinates of libraries to add to /lib/ dir from where the Arquillian extension below can shrinkwrap them into test wars.

* Contributors:
* IBM Corporation - initial API and implementation
*******************************************************************************/
package org.eclipse.microprofile.config12.tck;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TCK specific package name.

* There is a detailed output on specific
*/
@RunWith(FATRunner.class)
public class ConfigTckPackageTest {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TCK (or in this case spec) specific Test Class Name.


@AfterClass
public static void tearDown() throws Exception {
server.stopServer("CWMCG0007E", "CWMCG0014E", "CWMCG0015E", "CWMCG5003E", "CWWKZ0002E");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A list of specific expected errors your TCK leaves in the logs files. Error nopt in this list will cause the FAT bucket to be marked as failed.


@Test
@AllowedFFDC // The tested deployment exceptions cause FFDC so we have to allow for this.
public void testConfigTck() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the name of the testcase, the passing of which indicates ALL tests in the TCK passed. IN this case you might think it would be better as testConfig12TCK but that is fairly clear from the package name. Also - when experienced there is only a 'standard' failure message, so little point in triaging (double clicking on) this test - so in the longer tyerm it would be better to go ythe other way and have a really common recognisable name that was not spec or version spercific like testTCKKickoff - chat tomorrow Tom.

// mvn returns 0 if all surefire tests pass and -1 otherwise - this Assert is enough to mark the build as having failed
// the TCK regression

Assert.assertTrue("com.ibm.ws.microprofile.config.1.2_fat_tck:org.eclipse.microprofile.config12.tck.ConfigTckPackageTest:testTck:TCK has returned non-zero return code of: "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CSI requested a spec specific message here.

* Contributors:
* IBM Corporation - initial API and implementation
*******************************************************************************/
package org.eclipse.microprofile.config12.tck;
Copy link
Contributor

@hutchig hutchig Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bucket specific package.


@RunWith(Suite.class)
@SuiteClasses({
ConfigTckPackageTest.class
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bucket specific test class

<feature>webProfile-7.0</feature>
<feature>localConnector-1.0</feature>
<feature>cdi-1.2</feature>
<feature>mpConfig-1.2</feature>
Copy link
Contributor

@hutchig hutchig Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

features needed are often specific to TCK

<include location="../fatTestPorts.xml" />

<!--Java2 security-->
<javaPermission className="java.security.AllPermission" name="*" actions="*" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed for Arquillian getClassloader to be allowed in a Java 2 Security build.

xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>com.ibm.ws.microprofile.config12</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maven coords are FAT bucket specific

-->
<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >

<suite name="microprofile-config-1.2-tck" verbose="2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suite, test and package name are TCK specific

xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>com.ibm.ws.microprofile.config12</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

group and name are TCK specific

<name>MicroProfile Config 1.2 TCK Runner TCK Module</name>

<properties>
<microprofile.config.version>1.2</microprofile.config.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable name and the dependencies it is used in are TCK specific.

<artifactId>microprofile-config-api</artifactId>
<version>${microprofile.config.version}</version>
<scope>system</scope>
<systemPath>${com.ibm.websphere.org.eclipse.microprofile.config.1.2_}</systemPath>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the shipped API being tested.

<dependency>
<groupId>com.ibm.ws.org.jboss.arquillian.container</groupId>
<artifactId>arquillian-wlp-managed-8.5</artifactId>
<version>1.0.0.Final-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be possible that SOME TCKs will need a custom version of this jar until their requirements are met by either the IBM staging/common one (above) or the 'public' arquillian. If so, put it in artifactory with a different version name and reference it here.

@Override
public void process(Archive<?> applicationArchive, TestClass testClass) {
if (applicationArchive instanceof WebArchive) {
File hamcrest = new File("../../../lib/hamcrest-all-1.3.jar");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any additional jars used in the TCK that are not in LIberty can be added to build.gradle and referenced here.

public class ArquillianLoadableExtension implements LoadableExtension {
@Override
public void register(ExtensionBuilder extensionBuilder) {
System.out.println("WLP: Adding Extension com.ibm.ws.microprofile.config12.test.ArchiveProcessor");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, is class (name) specific to aid debugging.

@@ -0,0 +1 @@
com.ibm.ws.microprofile.config12.test.ArquillianLoadableExtension
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is project specific text too.

-->
<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >

<suite name="microprofile-config-1.2-tck" verbose="2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TCK specific

<method-selector>
<script language="beanshell">
<![CDATA[
!method.getDeclaringClass().getSimpleName().startsWith("CDIPlainInjectionTest") &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are tests that are being TEMPORARILY excluded as they don't pass.

</script>
</method-selector>
</method-selectors>
<package name="org.eclipse.microprofile.config.tck.*"></package>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TCK specific.

@@ -29,7 +26,6 @@
* This is a test class that runs a whole Maven TCK as one test FAT test.
* There is a detailed output on specific
*/
@SuppressWarnings("restriction")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to chat about this.

// ftInvalidCB5. The exception message was:
// com.ibm.ws.container.service.state.StateChangeException:
// org.jboss.weld.exceptions.DefinitionException
"CWMFT5003E", // CWMFT5003E: The fallback method fallbackForServiceB with parameter types
Copy link
Contributor

@hutchig hutchig Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't prefer this change to the layout

mvn = mvn + ".cmd";
}
mvnCliRaw = new String[] { mvn, "clean", "test", "-Dwlp=" + wlp, "-Dtck_server=" + server.getServerName(),
"-Dtck_port=" + server.getPort(PortType.WC_defaulthost), "-DtargetDirectory=" + logDir.getAbsolutePath() + "/tck" };
Copy link
Contributor

@hutchig hutchig Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Props.DIR_LOG still under 'results' directory or somewhere that is copied back to the main build results area from the child build?

Copy link
Contributor

@hutchig hutchig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neil is reviewing the other commits. Nothing major in the TCK ones. Please could you call logDir resultsDir as it only works if it is the junit results dir and they are not logs.

if (!init) {
init(server);
}
// Everything under autoFVT/results is collected from the child build machine
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not clear that logDir is 'autoFVT/results' dir

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also LOG_DIR has a specific different meaning as an environment variable: https://www.ibm.com/support/knowledgecenter/en/SSEQTP_8.5.5/com.ibm.websphere.wlp.doc/ae/rwlp_logging.html

int rc = runCmd(MvnUtils.mvnCliTckRoot, MvnUtils.tckRunnerDir, mvnOutput);
Assert.assertEquals("mvn returned non-zero: " + rc, 0, rc);
File src = new File(MvnUtils.logDir, "tck/surefire-reports/junitreports");
File tgt = new File(MvnUtils.logDir, "junit");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So logDir must be 'results' for this to work - so better to call it 'resultsDir none of these are logs after all.

Copy link
Contributor

@hutchig hutchig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you conform with Neils review for the impl requests I am happy to approve the TCK one :-) (With the pref for logDir -> resultsDir as LOG_DIR is something else: https://www.ibm.com/support/knowledgecenter/en/SSEQTP_8.5.5/com.ibm.websphere.wlp.doc/ae/rwlp_logging.html )

@tevans78 tevans78 force-pushed the 1551-runConfig12TCK branch from 7472a8e to f381ecb Compare January 24, 2018 13:09
@tevans78 tevans78 force-pushed the 1551-runConfig12TCK branch from f381ecb to 7e07175 Compare January 24, 2018 13:23
@tevans78
Copy link
Member Author

#build
fixes #1551

@LibbyBot
Copy link

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_njDmQAEBEei-3vT9-bzFwg

Target locations of links might be accessible only to IBM employees.

Copy link
Contributor

@hutchig hutchig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LibbyBot
Copy link

The build tevans78-1725-20180124-1335
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_njDmQAEBEei-3vT9-bzFwg
completed successfully!

@tevans78 tevans78 merged commit 6762bd3 into OpenLiberty:integration Jan 25, 2018
@tevans78 tevans78 deleted the 1551-runConfig12TCK branch December 11, 2018 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants