Fix exec_replace

[MrDenkoV]

Fix #125, Fix #129

[mkaput]

I am amazed that this fixes #129. Can you please properly link #129 to this issue? (you need to write fix #125, fix #129 in issue description)

Let's test this more thoroughly

[maciektr]

👋
On the topic of our offline conversation about ctrl_c test on windows 🪟 :

The error you have been seeing seems to come from the subcommand spawn.

This version of %1 is not compatible with the version of Windows you're running. Check your computer's system information and then contact the software publisher. (os error 216)

It seems that the script definition used there is just not runnable on windows. (

scarb/scarb/tests/subcommand.rs

Line 144 in f8494ed

formatdoc!(

)

After changing the script definition, I was able to run it successfully and ensure it has been killed by ctrl_c.
To test this, I have created a main.rs file in scarb-test-support crate with following content: https://gist.github.com/maciektr/6b6c18a358b5792912bc0d0261ef3ac2
Then I have used this in tests instead.

    let addr = {
        let addr = listener.local_addr().unwrap();
        addr.to_string()
    };

    let mut child = Scarb::new()
        .std()
        .arg("test-support")
        .arg("hang-on-tcp")
        .arg("--address")
        .arg(addr)

May not be the cleanest flow, but imho worth considering.

I have not been able to make this check work though. I do not understand what is the purpose of this.

    match sock.read(&mut [0; 10]) {
        Ok(n) => assert_eq!(n, 0),
        Err(e) => assert_eq!(e.kind(), io::ErrorKind::ConnectionReset),
    }

I know it has been copied from Cargo verbatim. Maybe it is enough for now to only check if the process has been killed? cc @mkaput

[mkaput]

I have not been able to make this check work though. I do not understand what is the purpose of this.

    match sock.read(&mut [0; 10]) {
        Ok(n) => assert_eq!(n, 0),
        Err(e) => assert_eq!(e.kind(), io::ErrorKind::ConnectionReset),
    }

I know it has been copied from Cargo verbatim. Maybe it is enough for now to only check if the process has been killed? cc @mkaput

This check tests if the socket either closed successfully without any unread rubbish left in the buffer, or died by the second party being terminated.

It seems that the script definition used there is just not runnable on windows. (

scarb/scarb/tests/subcommand.rs

Line 144 in f8494ed

formatdoc!(

)
After changing the script definition, I was able to run it successfully and ensure it has been killed by ctrl_c. To test this, I have created a main.rs file in scarb-test-support crate with following content: https://gist.github.com/maciektr/6b6c18a358b5792912bc0d0261ef3ac2 Then I have used this in tests instead.

    let addr = {
        let addr = listener.local_addr().unwrap();
        addr.to_string()
    };

    let mut child = Scarb::new()
        .std()
        .arg("test-support")
        .arg("hang-on-tcp")
        .arg("--address")
        .arg(addr)

May not be the cleanest flow, but imho worth considering.

I like this! I'd only rather use Command::new(cargo_bin!("scarb-test-support")) instead of Scarb::new().std(), seems more clear.