fix: propagate build error and check if js files exist

[LeNei]

Context & Description

This PR aims to fix the command tuono build not propagating errors. It also adds checks for commands executing scripts if said script exists.

Closes: #278

[Valerioageno]

Thanks for the PR! Changes look perfect!

I'd like to ask you also to introduce two test cases for checking that the scripts exist (checking the vite failure is too complex right now; same for the tuono dev command).

To do it, I'd like to ask you to:

1. rename crates/tuono/tests/cli_tests.rs in crates/tuono/tests/cli_build.rs
2. Add a test case for testing that the BUILD_JS_SCRIPT does not exist
3. Add a test case for testing that the BUILD_TUONO_CONFIG does not exist

Let me know if I can support you in any way!

[Valerioageno]

Hey @LeNei, I noticed that you added an empty commit. Is that what you meant?

[LeNei]

@Valerioageno the empty commit was a mistake when i updated my fork. I've added a test for running the cli without installed scripts but the separate cases are a bit tricky to tackle since the BUILD_TUONO_CONFIG script would have to run successfully before BUILD_JS_SCRIPT would run. Testing the functions would fail since currently std::process:exit is used to terminate the process. This leads to the test terminating if the function using it is called directly. Maybe it could be a good idea to create a common error Struct to handle errors or use a crate like anyhow for error handling?

[Valerioageno]

Definitely a great idea adding a better error handling!
How about adding a function in the TempTuonoProject struct - maybe something similar to add_route (tests/utils.rs) - to enable the test to programmatically add a mock file in node_modules/.bin/.. and then check if they exist?

[LeNei]

I've added a test using add_route since it is sufficient for creating the mock file. Feel free to have a lock. I've used a comparison on the output to check if the cli check on the file existing was successful since it directly checks if the cli is still working as expected.

[Valerioageno]

1. I'd rename then the method add_route into add_file since it doesn't make sense in this case the method meaning.
2. I'd check that without the files you get the expected error.

i.e.

fn it_fails_without_installed_build_scritp() {
    TempTuonoProject::new();

    let mut test_tuono_build = Command::cargo_bin("tuono").unwrap();
    test_tuono_build
        .arg("build")
        .assert()
        .failure()
        .stderr("Failed to find the build script. Please run `npm install`\n");
}

fn it_fails_without_installed_build_config_script() {
    TempTuonoProject::new();

    let mut test_tuono_build = Command::cargo_bin("tuono").unwrap();
    temp_tuono_project.add_route("./node_modules/.bin/tuono-build-prod");
    test_tuono_build
        .arg("build")
        .assert()
        .failure()
        .stderr("Failed to find the build script. Please run `npm install`\n");
}

3. I think there is no need of predicates in this case

[LeNei]

I've updated the tests. It was required to make the mock file executable for the first script check to pass but now it should be good.

[Valerioageno]

The PR looks good to me!
Please fix the pipelines so we can merge!

Thanks for your improvement