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

implement json protocol #2

Closed
missedone opened this issue Dec 15, 2015 · 11 comments
Closed

implement json protocol #2

missedone opened this issue Dec 15, 2015 · 11 comments

Comments

@missedone
Copy link
Contributor

testng eclipse plugin by default use remote testng objectSerialization protocol to transfer the message, but we suffered issue like http://github.com/cbeust/testng-eclipse/issues/91 due to missing serialVersionUUID; though we are fine once we explicitly set the serialVersionUUID in PR testng-team/testng#913, it'd be nice to experiment with new ison protocol

missedone referenced this issue in testng-team/testng-eclipse Dec 15, 2015
@missedone
Copy link
Contributor Author

Hi @cbeust , @juherr
before implementation, we need define the protocol spec.
since the json format is to transfer various message like GenericMessage, SuiteMessage, TestMessage, and TestResultMessage, when receive the data we need to know what's the type of the message hence know how to unmarshall the json data to java object.
since data is transferred via socket, basically the data packet should contain info like the message type and message data (anything more?)

to organize the packet, we can either use delimiter or length based packet format.
for example, for delimiter based packet format: one byte of message type + delimiter + json data + new line
or, length based packet format: one bye of message type + json data length + json data + EOF

either format is extensible.

I think the delimiter based packet is easy to implement.
what do you think, any additional need to be considered?

@juherr
Copy link
Member

juherr commented Dec 16, 2015

@missedone
I didn't have a look on how testng is working with its plugin. Could you explain it in few words?

Before the implementation, I think we should talk with IntelliJ and Netbeans too. Maybe they have some needs which could be added at the same time.

@cbeust
Copy link
Collaborator

cbeust commented Dec 16, 2015

I recently had to do this for Kobalt and I implemented a two pass JSON
format. Each payload has an id followed by a data field. That data field is
a string containing JSON (so JSON inside JSON).

The code reads the id and once it knows which command is being requested,
it passes the data field to that command, which then knows how to decode it.

Cédric

On Wed, Dec 16, 2015 at 6:12 PM, Julien Herr notifications@github.com
wrote:

@missedone https://github.com/missedone
I didn't have a look on how testng is working with its plugin. Could you
explain it in few words?

Before the implementation, I think we should talk with IntelliJ and
Netbeans too. Maybe they have some needs which could be added at the same
time.


Reply to this email directly or view it on GitHub
#2 (comment)
.

@missedone
Copy link
Contributor Author

@cbeust , the format sound good to me:)

@juherr , could you invite the IntelliJ, Netbeans guy into the discussion here if you know anyone, thanks

@missedone
Copy link
Contributor Author

for now, my only concern is, since this will introduce json lib to TestNG process, it is used to marshall/unmarshall the message, no matter which json lib we use, jackson, jettison, gson or naming a few, (for now, i'm using gson 2.5 in my test), this will make a chance for json lib version conflict with the one of user's project depends. just like plugin issue testng-team/testng-eclipse#111

one option is shading the json lib into testng-temote.jar, what do you see any better option?

@cbeust
Copy link
Collaborator

cbeust commented Dec 26, 2015

No, shading is probably the safest solution.

Cédric

On Sat, Dec 26, 2015 at 8:07 PM, Nick Tan notifications@github.com wrote:

for now, my only concern is, since this will introduce json lib to TestNG
process, it is used to marshall/unmarshall the message, no matter which
json lib we use, jackson, jettison, gson or naming a few, (for now, i'm
using gson 2.5 in my test), this will make a chance for json lib version
conflict with the one of user's project depends. just like plugin issue
testng-team/testng-eclipse#111
testng-team/testng-eclipse#111

one option is shading the json lib into testng-temote.jar, what do you see
any better option?


Reply to this email directly or view it on GitHub
#2 (comment)
.

cbeust added a commit that referenced this issue Dec 27, 2015
@juherr
Copy link
Member

juherr commented Dec 28, 2015

IntelliJ and Netbeans guys are invited.

@akozlova
Copy link

IntelliJ doesn't use testng-remote anymore as we bundle testng plugin (and it's impossible for users to download different plugins for different testng versions used in user's projects) and the old protocol failed to communicate if sender/receiver are from different versions (objectSerialization) and the very old string-protocol doesn't support new features.

I believe that junit team tries (at least they discussed the remote protocol) to solve similar problems and it would be perfect to communicate with them too (https://github.com/junit-team/junit-lambda).

@missedone
Copy link
Contributor Author

@juherr
thanks for creating the tickets to invite the IntelliJ and Netbeans guys:)

@akozlova

the old protocol failed to communicate if sender/receiver are from different versions (objectSerialization) and the very old string-protocol doesn't support new features.

that's exactly where the new protocol come from.

for now, we prefer to use json as the message format, in general, the message protocol spec looks like: {"type":<the_message_type>,"data":<the_message_payload_in_json>}, here're some examples:

  • Generic Message
{"type":1,"data":{"messageType":1,"suiteCount":1,"testCount":1}}
  • Suite Message
{"type":10,"data":{"suiteName":"Default suite","methodCount":0,"startSuiteRun":true,"excludedMethods":[]}}
  • Test Message
{"type":100,"data":{"testStart":true,"suiteName":"Default suite","testName":"Default test","testMethodCount":2,"
passedTestCount":0,"failedTestCount":0,"skippedTestCount":0,"successPercentageFailedTestCount":0}}
  • Test Result Message
{"type":1000,"data":{"messageType":1016,"suiteName":"Default suite","testName":"Default test","testClassName":"m
ymixed.GreetingTest","testMethodName":"greetingTest","startMillis":1451316147629,"endMillis":0,"parameters":[],"paramTypes":[],"testDescription":"","i
nvocationCount":1,"currentInvocationCount":0,"instanceName":"mymixed.GreetingTest"}}

we'd like to hear from you, so it's still opening for any advice or changes.

thanks

@akozlova
Copy link

Should this format work with old testng versions? Like separate jar ready for tool vendors to be bundled and passed into the test's VM (by now we assume that all jars passed to the clients VM should be 1.3 compatible)? Or would it be the part of testng.jar, so tool vendors would need to switch between protocols based on version of testng used by user?

@missedone
Copy link
Contributor Author

hi @akozlova
in PR #5 , i break down the testng remote project into modules by API version:

[INFO] Remote TestNG ..................................... SUCCESS [1.380s]
[INFO] TestNG - Remote - Common .......................... SUCCESS [0.026s]
[INFO] TestNG Remote for version [6.9.10) ................ SUCCESS [0.008s]
[INFO] TestNG Remote for version [6.9.7, 6.9.10) ......... SUCCESS [0.008s]
[INFO] TestNG Remote for version [6.5.1, 6.9.7) .......... SUCCESS [0.010s]

so, yes, this protocol works with old testng versions by adding testng-remote-common and versioned testng-remotex_x_x jars to testng JVM at IDE side testng-team/testng-eclipse#213 before launch the process.

all the jars compiled with Java7 so it's not 1.3 compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants