Sed 4562 support junit ap tests with keyword accessing the ap content for step 29#598
Conversation
Summary of ChangesHello @david-stephan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the local execution capabilities for JUnit Automation Package tests. It introduces a specialized operation mode for local automation packages and provides a new utility to dynamically archive classpath contents, allowing keywords to access resources within the running automation package. This change improves the flexibility and functionality of local test execution by ensuring that the necessary resources are correctly identified and made available. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces support for JUnit tests to access Automation Package (AP) content by simulating an AP from the current classpath. This is a significant improvement for testing keywords that rely on AP resources. The implementation uses a new OperationMode.LOCAL_AUTOMATION_PACKAGE and a LocalFunctionHandler to bundle the classpath into a temporary JAR. Feedback includes addressing efficiency and resource management in LocalFunctionHandler due to lack of caching, ensuring correct isLocalPackage flag usage, and resolving unused code and documentation mismatches in utility classes.
| public File retrieveAndExtractAutomationPackageFile(Map<String, String> properties) throws FileManagerException { | ||
| try { | ||
| // For local Function we artificially create an archive using the application classloader defined as system property | ||
| File tempFile = FileHelper.createTempFile(); | ||
| ClassLoaderArchiver.createArchive(tempFile, true); | ||
| return extractAutomationPackageFile(tempFile); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("Unable to create temporary archive file for the classloader", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation creates a new temporary JAR file and unzips it into a new temporary directory every time retrieveAndExtractAutomationPackageFile is called. Since FileHelper.createTempFile() returns a unique path each time, it bypasses the caching mechanism in extractAutomationPackageFile (which uses the file path as a key in the session). This results in significant performance overhead and disk space consumption.
Consider caching the generated archive file so it can be reused across multiple calls within the same execution context.
| //for local Keyword we always consider that we hare in the context of an AP | ||
| return true; | ||
| }} |
There was a problem hiding this comment.
Fix typo in comment ('hare' -> 'are') and remove double closing brace at the end of the file. The return true for isLocalPackage is appropriate here, as the local keyword context implies the automation package is part of the running process's classloader, aligning with the rule for setting isLocalPackage to true.
| //for local Keyword we always consider that we hare in the context of an AP | |
| return true; | |
| }} | |
| //for local Keyword we always consider that we are in the context of an AP | |
| return true; | |
| } | |
| } |
References
- The
isLocalPackageflag should be set totrueonly when the automation package is part of the running process's classloader (e.g., for a JUnit runner). For external packages, such as those used in CLI executions, it should befalse.
| */ | ||
| public class ClassLoaderArchiver { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(ClassLoaderArchiver.class); |
There was a problem hiding this comment.
The logger field is defined but never used in this class.
|
|
||
| /** | ||
| * Creates a JAR archive at {@code outputFile} that contains all classes and | ||
| * resources reachable from {@code classLoader}. |
There was a problem hiding this comment.
The Javadoc mentions a classLoader parameter that is not present in the method signature. The method currently uses the system property java.class.path directly.
| private static boolean containsClassesDirectory(Set<File> files) { | ||
| return files.stream().anyMatch(f -> f.isDirectory() && | ||
| (f.getPath().contains("classes") || f.getPath().contains("resources"))); | ||
| } |
There was a problem hiding this comment.
The method containsClassesDirectory is unused and can be removed.
|
|
||
| @Override | ||
| public boolean containsAutomationPackageFileReference(Map<String, String> properties) { | ||
| //for local Keyword we always consider that we hare in the context of an AP |
There was a problem hiding this comment.
| //for local Keyword we always consider that we hare in the context of an AP | |
| //for local Keyword we always consider that we are in the context of an AP |
david-stephan
deleted the
SED-4562-support-junit-ap-tests-with-keyword-accessing-the-ap-content-for-step-29
branch
… for step 29 (#598) * SED-4555 Local execution of AP with Keyword accessing the AP content is not supported * SED-4555 PR feedback and cleanup * SED-4555 ClassLoaderArchiver must exclude signature files * SED-4555 switching to sequential unzip * SED-4555 PR feedbacks * SED-4562 Support Junit AP tests with Keyword accessing the AP content for Step 29 * SED-4562 PR feedbacks * SED-4562 fixing cases where operation mode is null in execution context
2 participants
No description provided.