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

[SUREFIRE-1222] ForkClient attempts to consume unrelated lines #210

Closed
wants to merge 1 commit into from

Conversation

eolivelli
Copy link
Contributor

This is a WIP pull request, to help reviews on the branch for
https://issues.apache.org/jira/browse/SUREFIRE-1222

@Tibor17
Copy link
Contributor

Tibor17 commented Dec 15, 2018

@eolivelli
I am looking forward to spend Christmas time on this with you as well. ;-)
A bit crazy to say that, isn't it, but that's the reality in OSS.

@Tibor17
Copy link
Contributor

Tibor17 commented Dec 15, 2018

We should not forget that the following branch [1] is part of interprocess communication. The TestNG Suites should be determined by type, either XML, File or array. This includes tests which should precisely specify what is sent, either a class or a suite and type. This would mean that Surefire will be Open-Closed Principle together with plugin Extensions and the user can write his own Booter.
[1]: https://gitbox.apache.org/repos/asf?p=maven-surefire.git;a=shortlog;h=refs/heads/V30_SUREFIRE-1535

Copy link
Contributor Author

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tibor17 this branch does not compile
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project surefire-api: Compilation failure: Compilation failure:
[ERROR] /home/eolivelli/dev/maven-surefire/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java:[53,1] cannot find symbol
[ERROR] symbol: static encodeStringForForkCommunication
[ERROR] location: class
[ERROR] /home/eolivelli/dev/maven-surefire/surefire-api/src/main/java/org/apache/maven/surefire/booter/ForkingRunListener.java:[42,1] cannot find symbol
[ERROR] symbol: static escapeToPrintable
[ERROR] location: class
[ERROR] /home/eolivelli/dev/maven-surefire/surefire-api/src/main/java/org/apache/maven/surefire/booter/ForkingRunListener.java:[41,1] cannot find symbol
.......

* <p/>
*
* @author <a href="mailto:tibordigana@apache.org">Tibor Digana (tibor17)</a>
* @since 2.20.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tibor17 we will have to change all of these comments


private void encodeAndPrintEvent( StringBuilder command )
{
byte[] array = command.append( '\n' ).toString().getBytes( streamCharset );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is better to write the length of the command and then the command and do not rely on a separator

encodeBytes = new byte[len];
arraycopy( buf, off, encodeBytes, 0, len );
}
return encodeMessage( operation, runMode, bufEncoding, printBase64Binary( encodeBytes ) );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general I would prefer to use framing ( length + payload) instead of encoding with base64
This way the protocol is much efficient and most of the data flow thru the socket without modifications (with a sniffer is easier to perform debug).
on the reader's side you can pre-allocate buffers and do very useful things with framing

@asfgit asfgit closed this Mar 27, 2019
@asfgit asfgit deleted the SUREFIRE-1222 branch March 27, 2019 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants