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

fix filename used in ConcurrencySigTest #195

Merged
merged 2 commits into from Mar 25, 2022

Conversation

aubi
Copy link
Contributor

@aubi aubi commented Mar 24, 2022

Recent commit (19cfa43) changed constant for signature file to contain version number. Deployment was adapted, but it wasn't reflected in the test.

Copy link
Contributor

@njr-11 njr-11 left a comment

Choose a reason for hiding this comment

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

I noticed this too when I tried to run the TCK after pulling in those changes. I think we should go even further and get rid of the 3.0.0 from the file name altogether. The less places we have a hard-coded version the better. That will make it easier on developers for future Concurrency spec releases.

@aubi
Copy link
Contributor Author

aubi commented Mar 25, 2022

@njr-11 Like this? I absolutely agree -- the code became a bit simpler.
There is one more leftover -- in the sig-test.map. But if I will remove it, it will be empty. In the code, a key "jakarta.transaction" is loaded from it, so I left it unchanged.

@njr-11
Copy link
Contributor

njr-11 commented Mar 25, 2022

@njr-11 Like this? I absolutely agree -- the code became a bit simpler. There is one more leftover -- in the sig-test.map. But if I will remove it, it will be empty. In the code, a key "jakarta.transaction" is loaded from it, so I left it unchanged.

Yes, that looks great. I'll go ahead and merge this for now so that we have a working TCK again.

The "jakarta.transaction" logic seems to be intended for signature testing for the JTA spec, and I don't see any reason why signature testing for Jakarta Concurrency should include it. I think we can remove it in a separate pull.

				boolean isJTASigTest = false;

				// Determine whether the signature map file contains package
				// jakarta.transaction
				String jtaVersion = mapFileAsProps.getProperty("jakarta.transaction");
				if (jtaVersion == null || "".equals(jtaVersion.trim())) {
					System.out.println("SigTestEE.signatureTest() returning, "
							+ "as this is neither JTA TCK run, not Java EE CTS run.");
					return;
				}

				System.out.println("jtaVersion " + jtaVersion);
				// Signature map packaged in JTA TCK will contain a single package
				// jakarta.transaction
				if (mapFileAsProps.size() == 1) {
					isJTASigTest = true;
				}

				if (isJTASigTest || !jtaVersion.startsWith("1.2")) {
					verifyJtaJarTest();
				}
	public void verifyJtaJarTest() throws Exception {
		System.out.println("SigTestEE#verifyJtaJarTest - Starting:");
		String repositoryDir = getRepositoryDir();
		String jtaJarClasspath = testInfo.getJtaJarClasspath();
		boolean result = getSigTestDriver().verifyJTAJarForNoXA(testInfo.getJtaJarClasspath(), repositoryDir);
		if (result) {
			System.out.println("PASS: javax.transaction.xa not found in API jar");
		} else {
			System.out.println("FAIL: javax.transaction.xa found in API jar");
			throw new Fault("javax.transaction.xa validation failed");
		}
		System.out.println("SigTestEE#verifyJtaJarTest returning");
	}
	public void verifyJtaJarTest() throws Exception {
		System.out.println("SigTestEE#verifyJtaJarTest - Starting:");
		String repositoryDir = getRepositoryDir();
		String jtaJarClasspath = testInfo.getJtaJarClasspath();
		boolean result = getSigTestDriver().verifyJTAJarForNoXA(testInfo.getJtaJarClasspath(), repositoryDir);
		if (result) {
			System.out.println("PASS: javax.transaction.xa not found in API jar");
		} else {
			System.out.println("FAIL: javax.transaction.xa found in API jar");
			throw new Fault("javax.transaction.xa validation failed");
		}
		System.out.println("SigTestEE#verifyJtaJarTest returning");
	}

@njr-11 njr-11 merged commit ee1b7b6 into jakartaee:master Mar 25, 2022
@njr-11
Copy link
Contributor

njr-11 commented Mar 25, 2022

#197 removes the version from sig-test.map as discussed above. The TCK runs cleanly with that change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants