Remove Dead Code from the Android TestController #4638
Conversation
| if (_class == null) { | ||
| return null; | ||
| } | ||
| return _class.newInstance(); |
There was a problem hiding this comment.
Class::newInstance has been deprecated for years:
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Class.html#newInstance()
Class::getDeclaredConstructor is the proper way to do this in Java.
| java.net.NetworkInterface iface = ifaces.nextElement(); | ||
| java.util.Enumeration<java.net.InetAddress> addrs = iface.getInetAddresses(); | ||
| while (addrs.hasMoreElements()) { | ||
| java.net.InetAddress addr = addrs.nextElement(); |
There was a problem hiding this comment.
No changes, just an intermediate variable to make it easier to read.
| initData.properties.setProperty("ControllerAdapter.PublishedHost", "127.0.0.1"); | ||
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
Again, no changes, just better formatting.
| initData.properties.setProperty("IceDiscovery.DomainId", "TestController"); | ||
| } | ||
|
|
||
| _communicator = new com.zeroc.Ice.Communicator(initData); |
There was a problem hiding this comment.
All the dead code:
|
|
||
| public void communicatorInitialized(Communicator communicator) {} | ||
|
|
||
| public java.io.InputStream loadResource(String path) { |
There was a problem hiding this comment.
Just more minor formatting improvements for the rest of the file.
There was a problem hiding this comment.
Pull Request Overview
This PR removes the deprecated ProcessControllerRegistry interface and its associated implementation code from the test infrastructure. The registry mechanism was previously used for bidirectional communication between the test controller and remote process controllers, but has been replaced by a simpler discovery-based approach.
- Removed
ProcessControllerRegistryinterface from Controller.ice - Cleaned up Python and Java implementations by removing registry-related code
- Updated deprecated Java API usage (
newInstance()togetDeclaredConstructor().newInstance())
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/Controller.ice | Removed ProcessControllerRegistry interface definition |
| scripts/Util.py | Removed adapter initialization, ProcessControllerRegistryI class, and setProcessController method; simplified controller ping logic; fixed typo in comment |
| java/test/android/controller/src/main/java/com/zeroc/testcontroller/ControllerApp.java | Removed unused imports and registry-related methods (registerProcessController, handleException); updated deprecated API calls; improved code formatting |
| java/BUILDING.md | Fixed command to include .py extension for allTests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| com.zeroc.Ice.Util.stringToIdentity( | ||
| "Android/ProcessController"))); | ||
| com.zeroc.Ice.ObjectAdapter adapter = _communicator.createObjectAdapter("ControllerAdapter"); | ||
| adapter.add(new ProcessControllerI(), com.zeroc.Ice.Util.stringToIdentity("Android/ProcessController")); |
There was a problem hiding this comment.
I would replace com.zeroc.Ice.Util.stringToIdentity with Identity constructor.
| def teardown(self, current, success): | ||
| pass | ||
|
|
||
| def __init__(self, current, endpoints): |
There was a problem hiding this comment.
Endpoints can also be removed from this constructor now as well.
|
|
||
| import Ice | ||
|
|
||
| comm.getProperties().setProperty("Adapter.AdapterId", str(uuid.uuid4())) |
There was a problem hiding this comment.
You can delete this property as well.
InsertCreativityHere
force-pushed
the
android-cleanup
branch
from
b52d67d to
3a2dfd9
Compare
InsertCreativityHere
force-pushed
the
android-cleanup
branch
from
df6a1c1 to
0170306
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
This PR removes all the leftover code from back when we were using bidir for the Android tests.
See #4587