starfixdev / starfix

Utility to easily open and operator on source code via url links from your browser
Apache License 2.0
26 stars 21 forks source link

Fixed test failure in windows ie, `testEcho()` method #119

Open Kshitiz-Mhto opened 1 year ago

Kshitiz-Mhto commented 1 year ago

part of fixing #104

fahad-israr commented 1 year ago

Didn't get u here. Thought issue was around some text formatting? What was the actual isssue acc to you? How does using a process builder instead of process executor fix it?

Kshitiz-Mhto commented 1 year ago

i thought the same first, that issue is causing due to some text formatting in windows. But using ProcessExecutor to run external command in my windows machine i am getting such error msg:

<testcase name="testEcho" classname="dev.starfix.StarfixTest" time="0.563">
    <error message="Could not execute [echo, This is some random String that I want to Echo]. Error=2, The system cannot find the file specified" type="org.zeroturnaround.exec.ProcessInitException">org.zeroturnaround.exec.ProcessInitException: Could not execute [echo, This is some random String that I want to Echo]. Error=2, The system cannot find the file specified
    at dev.starfix.StarfixTest.testEcho(StarfixTest.java:42)
Caused by: java.io.IOException: Cannot run program "echo": CreateProcess error=2, The system cannot find the file specified
    at dev.starfix.StarfixTest.testEcho(StarfixTest.java:42)
Caused by: java.io.IOException: CreateProcess error=2, The system cannot find the file specified
    at dev.starfix.StarfixTest.testEcho(StarfixTest.java:42)
</error>

the error is indicating it isnot able to find or execute the executable echo executable in windows.

so i went for the alternative choice using ProcessBuilder to run external command and it worked fine...

fahad-israr commented 1 year ago

can u plz try figuring out what's the issue with ProcessExecutor here? we should have a very valid reason when discarding ProcessExecutor and moving to ProcessBuilder imo .

Kshitiz-Mhto commented 1 year ago

can u plz try figuring out what's the issue with ProcessExecutor here? we should have a very valid reason when discarding ProcessExecutor and moving to ProcessBuilder imo .

no problem, i m going to figure this out.

Kshitiz-Mhto commented 1 year ago

@fahad-israr done 👍. I fixed it using ProcessExecutor. just one doubt, why we are using 3rd party library ProcessExecutor other than using built in library ProcessBuilder class for performing simpler task like for echo command? Is there specific reason to it?

fahad-israr commented 1 year ago
maxandersen commented 1 year ago

Zut process executor is afaik still the best around when it comes to manage the handling of input and output streams.

It takes care of the threads currently today required to be used to get output and empty/close the streams.

Which you need to do especially on windows where a process won't even execute if you don't continuously empty the stream.