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

TCK: Standardize test clients and context lookups #292

Merged
merged 4 commits into from
Jul 10, 2023

Conversation

KyleAure
Copy link
Contributor

Related to issue #269

There were a number of TCK test classes that just call tests within a servlet and nothing more.
In these cases, the test class can act both as a client and a servlet so we don't need all the extra boilerplate code.

Additionally, I standardized the use of @Resource and InitalContext.doLookup().

@KyleAure KyleAure added enhancement New feature or request TCK labels Jun 27, 2023
@KyleAure KyleAure self-assigned this Jun 27, 2023
@KyleAure KyleAure requested a review from mswatosh June 28, 2023 14:31
@@ -42,6 +44,9 @@ public static WebArchive createDeployment() {
return ShrinkWrap.create(WebArchive.class)
.addPackages(true, ContextServiceTests.class.getPackage());
}

@Resource(lookup = TestConstants.DefaultContextService)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to explicitly specify default values? I would use the lookup only when needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In some cases, yes.
This resource is injected by the Arquillian enricher and not the application server itself.

@KyleAure
Copy link
Contributor Author

KyleAure commented Jul 7, 2023

I'll leave this PR open for a bit longer to see if there are any review comments.

@KyleAure KyleAure merged commit 9db2339 into jakartaee:main Jul 10, 2023
@KyleAure KyleAure deleted the 269-remove-arquillian-clients branch July 18, 2023 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request TCK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants