-
Notifications
You must be signed in to change notification settings - Fork 1k
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
remove remote/distributed classes #953
Conversation
cd19b11
to
4555db3
Compare
* @return | ||
* @since 6.9.11 when moving distributed/remote classes out into separate project | ||
*/ | ||
protected List<ISuite> runSuites() { |
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.
introduce new override-able method runSuites
to abstract how to run the suites, this is helpful for DistributedTestNG to override this method to run master/slave mode according to command line args, see also: https://github.com/testng-team/testng-remote/blob/rm_distributed_classes/distributed/distributed/src/main/java/org/testng/distributed/DistributedTestNG.java#L54
That's a huge change. If you're telling me it's mostly files being moved around, I'll take your work and I'll merge it (pending @juherr's approval). |
yes, most of them just being moved. |
@@ -1577,8 +1555,6 @@ public void configure(Map cmdLineArgs) { | |||
result.xmlPathInJar = (String) cmdLineArgs.get(CommandLineArgs.XML_PATH_IN_JAR); | |||
result.junit = (Boolean) cmdLineArgs.get(CommandLineArgs.JUNIT); | |||
result.mixed = (Boolean) cmdLineArgs.get(CommandLineArgs.MIXED); | |||
result.master = (String) cmdLineArgs.get(CommandLineArgs.MASTER); |
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.
Some warnings/exceptions are missing here to say that the master/slave feature moved into another jar.
Everything seems good if we don't miss to transfert some files ;) But merging this PR will have 1 big side effect: testng-team/testng-remote#1 will have to be closed before the next TestNG release. Without it, we won't have workarround for the removed features. |
* | ||
* @author Cedric Beust <[email protected]> | ||
*/ | ||
public class ResultContentHandler extends DefaultHandler { |
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.
I don't find it on https://github.com/testng-team/testng-remote/tree/rm_distributed_classes
Check: Except 2 classes, I found all others. |
thanks @juherr i just added the two missing classes here: note, for compatibility purpose, i modified ResultXMLParser.java to call parse method with reflection. i'm wondering is ResultXMLParser still in used? i don't see any reference to it, @cbeust |
Looks like we're ready to merge, @missedone can you do a rebase? If not I'll merge manually. |
4555db3
to
a6742ea
Compare
hi @cbeust |
remove remote/distributed classes
moving the remote/distributed TestNG classes to separate project.
basically, this is prototype how we can remote distributed testng out.
see supplementary PR here: testng-team/testng-remote#14