fix(cli/test): fix clear screen behavior when run deno test --watch

[liruifengv]

fix #19725

[bartlomieju]

Thanks for the PR @liruifengv, could you update some of unit tests in cli/args/flags.rs to make sure we don't have a regression in the future?

[liruifengv]

Thanks for the PR @liruifengv, could you update some of unit tests in cli/args/flags.rs to make sure we don't have a regression in the future?↳

I'd be happy to update the unit tests, but could you give a little more hints, I've just changed an incorrect bool value being passed, and I'm not quite sure how to update the unit tests.

[bartlomieju]

There's test_watch test in cli/args/flags.rs - make sure that it has correct value for the "no clear screen" argument. That should be enough :)

[liruifengv]

I don't think this test needs updating, it's correct

[bartlomieju]

I don't think this test needs updating, it's correct

If that test was correct then why the behavior reported in the issue wasn't?

[liruifengv]

This incorrect behavior is that the screen is not cleared when the --no-clear-screen flag is not passed by default, and the screen is cleared when the --no-clear-screen flag is passed true.

This is because we passed the opposite value to the file_watcher::watch_func function, incorrectly using ! for the inverse.

  file_watcher::watch_func(
    flags,
    file_watcher::PrintConfig {
      job_name: "Test".to_string(),
      clear_screen: !test_flags
        .watch
        .as_ref()
        .map(|w| !w.no_clear_screen)
        .unwrap_or(true),
    },
    ...
  )
  .await?;

So much so that watch_func gets the wrong PrintConfig.clear_screen parameter, performing the opposite action.

The value of test_flags is correct, so test was also correct.

[bartlomieju]

This incorrect behavior is that the screen is not cleared when the --no-clear-screen flag is not passed by default, and the screen is cleared when the --no-clear-screen flag is passed true.

This is because we passed the opposite value to the file_watcher::watch_func function, incorrectly using ! for the inverse.

  file_watcher::watch_func(
    flags,
    file_watcher::PrintConfig {
      job_name: "Test".to_string(),
      clear_screen: !test_flags
        .watch
        .as_ref()
        .map(|w| !w.no_clear_screen)
        .unwrap_or(true),
    },
    ...
  )
  .await?;

So much so that watch_func gets the wrong PrintConfig.clear_screen parameter, performing the opposite action.

The value of test_flags is correct, so test was also correct.

Oh, got it, sorry for the confusion. In such case - maybe we could adjust the integration tests in cli/tests/integration/watcher_tests.rs (test_watch_basic) and check that the CLEAR_SCREEN character is printed?

[liruifengv]

I think so. But I found this pr #17129 which does not clear screen in non-TTY environments. see also
https://github.com/denoland/deno/blob/main/cli/tests/integration/watcher_tests.rs#L554

[liruifengv]

I can also write the test code:

 let line = next_line(&mut stderr_lines).await.unwrap();
  assert_not_contains!(&line, CLEAR_SCREEN);
  assert_contains!(&line, "Restarting");

But I don't think it's meaningful because it always not_contains in non-TTY environments.

[bartlomieju]

Okay, let's land it as is.