-
-
Notifications
You must be signed in to change notification settings - Fork 153
Add remote test execution support for Python/Behave backend with generic message endpoint #579
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
base: main
Are you sure you want to change the base?
Conversation
…t execution Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
laeubi
left a comment
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.
@copilot some adjustments are needed see comments below.
| * | ||
| * @author copilot | ||
| */ | ||
| public class BehaveMessageEndpointProcess implements IProcess, EnvelopeProvider, ISuspendResume, IDisconnect { |
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.
It looks like this code is generic enough that it can be shared between java+behave, we should move it to the generic eclipse.editor plugin instead.
to make it even more generic instead we should:
- instead of
Activator.getDefault().getLog()we should use ILog.error(...) - have a getter method that returns the choosen port so it can generically be accessed
- the implementation sepecific "add arguments" methods can then be moved to a backend specific part
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.
Moved the generic message endpoint code to io.cucumber.eclipse.editor.launching.MessageEndpointProcess. The Python bundle now extends this class with just ~33 lines of backend-specific code. Changes:
- Changed
Activator.getDefault().getLog()toILog.get() - Added
getPort()getter method - Backend-specific
addArgumentsmoved toBehaveProcessLauncher.withRemoteConnection()
Commit: 7254fc6
...er.eclipse.python/src/io/cucumber/eclipse/python/launching/BehaveMessageEndpointProcess.java
Outdated
Show resolved
Hide resolved
...thon/src/io/cucumber/eclipse/python/launching/CucumberBehaveLaunchConfigurationDelegate.java
Outdated
Show resolved
Hide resolved
… for port Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
@copilot I noticed in this message you try to access |
Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
... The HTTP URL was in the target definition file at Commit: 15e6240 |
io.cucumber.eclipse.editor/src/io/cucumber/eclipse/editor/launching/MessageEndpointProcess.java
Show resolved
Hide resolved
Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
laeubi
left a comment
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.
@copilot just two small suggestions, see below.
| private Map<String, String> attributes = new HashMap<>(); | ||
| private volatile boolean suspended; | ||
| private IBreakpoint[] breakpoints; | ||
| public class MessageEndpointProcess extends io.cucumber.eclipse.editor.launching.MessageEndpointProcess { |
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 we rename it to
| public class MessageEndpointProcess extends io.cucumber.eclipse.editor.launching.MessageEndpointProcess { | |
| public class JavaMessageEndpointProcess extends io.cucumber.eclipse.editor.launching.MessageEndpointProcess { |
to prevent the name clash?
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.
Renamed the class to JavaMessageEndpointProcess to prevent name clash with the base class. Updated all references in CucumberFeatureLocalApplicationLaunchConfigurationDelegate.
Commit: 730d7d1
| @Override | ||
| protected Envelope deserializeEnvelope(byte[] buffer, int length) throws IOException { | ||
| return Jackson.OBJECT_MAPPER.readerFor(Envelope.class).readValue(buffer, 0, length); | ||
| } |
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.
Both java + behave seem using the same code here, we should move it to the base class as it is likely always they case and implementations can still override it if really needed to.
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.
Moved the common deserializeEnvelope() implementation to the base class in io.cucumber.eclipse.editor.launching.MessageEndpointProcess. Both Java and Python now use this shared implementation. Added dependency on io.cucumber.eclipse.java.plugins to the editor bundle for Jackson access.
Commit: 730d7d1
…ialization to base class Co-authored-by: laeubi <1331477+laeubi@users.noreply.github.com>
Final Refactoring Updates
Renamed Java Implementation
MessageEndpointProcesstoJavaMessageEndpointProcessto prevent name clash with the generic base classCucumberFeatureLocalApplicationLaunchConfigurationDelegateMoved Common Code to Base Class
deserializeEnvelope()implementation to the base class since both Java and Python use identical Jackson-based deserializationgetLabel()and provide backend-specific argument injectionio.cucumber.eclipse.java.pluginsto editor bundle for Jackson accessBackend-Specific Code
addArguments()to inject-pplugin argumentBehaveProcessLauncher.withRemoteConnection()for argument injectionCode Reduction
The refactoring demonstrates maximum code reuse while maintaining clean separation of backend-specific concerns.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.