Skip to content

Commit

Permalink
refactor: Clean up test code and don't wait if unnecessary
Browse files Browse the repository at this point in the history
  • Loading branch information
Nukesor committed Jan 21, 2025
1 parent 5a46853 commit d86b9c8
Show file tree
Hide file tree
Showing 30 changed files with 257 additions and 159 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ This release aims to further improve Pueue and to rectify some old design decisi

### Removing internal channel communication

TLDR: Commands that start/stop/kill/pause tasks now only return when the task is actually started/stopped/killed/paused.
TLDR: Commands that start/stop/pause tasks now only return when the task is actually started/stopped/paused.
`kill`ing commands still takes a short while, as the process needs to be cleaned up properly.

---

Expand Down
2 changes: 1 addition & 1 deletion pueue/src/daemon/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub fn build_callback_command(
// This includes how many stashed and queued tasks are left in the group.
parameters.insert("group", task.group.clone());
let queued_tasks = state
.filter_tasks_of_group(|task| task.is_queued(), &task.group)
.filter_tasks_of_group(Task::is_queued, &task.group)
.matching_ids
.len();
parameters.insert("queued_count", queued_tasks.to_string());
Expand Down
16 changes: 9 additions & 7 deletions pueue/src/daemon/network/message_handler/kill.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use pueue_lib::state::SharedState;
use pueue_lib::success_msg;
use pueue_lib::{network::message::*, settings::Settings};
use pueue_lib::{
network::message::*, settings::Settings, state::SharedState, success_msg, task::Task,
};

use crate::daemon::network::response_helper::{ensure_group_exists, task_action_response_helper};
use crate::daemon::process_handler;
use crate::daemon::{
network::response_helper::{ensure_group_exists, task_action_response_helper},
process_handler,
};

/// Invoked when calling `pueue kill`.
/// Forward the kill message to the task handler, which then kills the process.
Expand All @@ -23,7 +25,7 @@ pub fn kill(settings: &Settings, state: &SharedState, message: KillMessage) -> M
TaskSelection::TaskIds(task_ids) => task_action_response_helper(
"Tasks are being killed",
task_ids.clone(),
|task| task.is_running(),
Task::is_running,
&state,
),
TaskSelection::Group(group) => {
Expand All @@ -38,7 +40,7 @@ pub fn kill(settings: &Settings, state: &SharedState, message: KillMessage) -> M
TaskSelection::TaskIds(task_ids) => task_action_response_helper(
"Tasks are being killed",
task_ids.clone(),
|task| task.is_running(),
Task::is_running,
&state,
),
TaskSelection::Group(group) => success_msg!(
Expand Down
14 changes: 8 additions & 6 deletions pueue/src/daemon/network/message_handler/restart.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use chrono::Local;
use pueue_lib::settings::Settings;
use std::sync::MutexGuard;

use pueue_lib::aliasing::insert_alias;
use pueue_lib::network::message::*;
use pueue_lib::state::{SharedState, State};
use pueue_lib::task::TaskStatus;
use pueue_lib::{
aliasing::insert_alias,
network::message::*,
settings::Settings,
state::{SharedState, State},
task::{Task, TaskStatus},
};

use crate::daemon::process_handler;

Expand All @@ -28,7 +30,7 @@ pub fn restart_multiple(
let response = task_action_response_helper(
"Tasks has restarted",
task_ids.clone(),
|task| task.is_done(),
Task::is_done,
&state,
);

Expand Down
2 changes: 1 addition & 1 deletion pueue/src/daemon/network/response_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn ensure_group_exists<'state>(
/// task_action_response_helper(
/// "Tasks are being killed",
/// task_ids.clone(),
/// |task| task.is_running(),
/// Task::is_running,
/// &state,
/// ),
/// ```
Expand Down
16 changes: 9 additions & 7 deletions pueue/src/daemon/process_handler/kill.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use log::{error, info, warn};

use pueue_lib::network::message::{Signal, TaskSelection};
use pueue_lib::process_helper::*;
use pueue_lib::settings::Settings;
use pueue_lib::state::GroupStatus;
use pueue_lib::task::TaskStatus;
use pueue_lib::{
network::message::{Signal, TaskSelection},
process_helper::*,
settings::Settings,
state::GroupStatus,
task::{Task, TaskStatus},
};

use crate::daemon::state_helper::{save_state, LockedState};
use crate::ok_or_shutdown;
Expand Down Expand Up @@ -124,7 +126,7 @@ fn should_pause_group(state: &LockedState, issued_by_user: bool, group: &str) ->
return false;
}

// Check if there're tasks that're queued or enqueued.
let filtered_tasks = state.filter_tasks_of_group(|task| task.is_queued(), group);
// Check if there're tasks that're queued or scheduled to be enqueued.
let filtered_tasks = state.filter_tasks_of_group(Task::is_queued, group);
!filtered_tasks.matching_ids.is_empty()
}
5 changes: 3 additions & 2 deletions pueue/tests/client/integration/env.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use anyhow::Result;
use pueue_lib::task::Task;

use crate::client::helper::*;

Expand All @@ -16,7 +17,7 @@ async fn set_environment() -> Result<()> {

// Now start the command and wait for it to finish
run_client_command(shared, &["enqueue", "0"])?;
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let state = get_state(shared).await?;
println!("{:#?}", state.tasks[&0].envs);
Expand Down Expand Up @@ -46,7 +47,7 @@ async fn unset_environment() -> Result<()> {

// Now start the command and wait for it to finish
run_client_command(shared, &["enqueue", "0"])?;
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let state = get_state(shared).await?;
println!("{:#?}", state.tasks[&0].envs);
Expand Down
8 changes: 5 additions & 3 deletions pueue/tests/client/integration/follow.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use anyhow::{Context, Result};
use rstest::rstest;

use pueue_lib::task::Task;

use crate::client::helper::*;

pub fn set_read_local_logs(daemon: &mut PueueDaemon, read_local_logs: bool) -> Result<()> {
Expand All @@ -27,7 +29,7 @@ async fn default(#[case] read_local_logs: bool) -> Result<()> {

// Add a task and wait until it started.
assert_success(add_task(shared, "sleep 1 && echo test").await?);
wait_for_task_condition(shared, 0, |task| task.is_running()).await?;
wait_for_task_condition(shared, 0, Task::is_running).await?;

// Execute `follow`.
// This will result in the client receiving the streamed output until the task finished.
Expand All @@ -51,7 +53,7 @@ async fn last_lines(#[case] read_local_logs: bool) -> Result<()> {

// Add a task which echos 8 lines of output
assert_success(add_task(shared, "echo \"1\n2\n3\n4\n5\n6\n7\n8\" && sleep 1").await?);
wait_for_task_condition(shared, 0, |task| task.is_running()).await?;
wait_for_task_condition(shared, 0, Task::is_running).await?;

// Follow the task, but only print the last 4 lines of the output.
let output = run_client_command(shared, &["follow", "--lines=4"])?;
Expand Down Expand Up @@ -123,7 +125,7 @@ async fn fail_on_non_existing(#[case] read_local_logs: bool) -> Result<()> {
//
// // Add a task echoes something and waits for a while
// assert_success(add_task(shared, "echo test && sleep 20").await?);
// wait_for_task_condition(shared, 0, |task| task.is_running()).await?;
// wait_for_task_condition(shared, 0, Task::is_running).await?;
//
// // Reset the daemon after 2 seconds. At this point, the client will already be following the
// // output and should notice that the task went away..
Expand Down
13 changes: 7 additions & 6 deletions pueue/tests/client/integration/log.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use std::collections::{BTreeMap, HashMap};

use anyhow::{Context, Result};
use pueue_lib::task::Task;
use rstest::rstest;
use serde::Deserialize;

use pueue_lib::task::Task;

use crate::client::helper::*;

/// Test that the `log` command works for both:
Expand All @@ -28,7 +29,7 @@ async fn read(#[case] read_local_logs: bool) -> Result<()> {

// Add a task and wait until it finishes.
assert_success(add_task(shared, "echo test").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let output = run_client_command(shared, &["log"])?;

Expand Down Expand Up @@ -59,7 +60,7 @@ async fn read_truncated(#[case] read_local_logs: bool) -> Result<()> {

// Add a task and wait until it finishes.
assert_success(add_task(shared, "echo '1\n2\n3\n4\n5\n6\n7\n8\n9\n10'").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let output = run_client_command(shared, &["log", "--lines=5"])?;

Expand All @@ -77,7 +78,7 @@ async fn task_with_label() -> Result<()> {

// Add a task and wait until it finishes.
run_client_command(shared, &["add", "--label", "test_label", "echo test"])?;
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let output = run_client_command(shared, &["log"])?;

Expand All @@ -95,7 +96,7 @@ async fn colored() -> Result<()> {

// Add a task and wait until it finishes.
assert_success(add_task(shared, "echo test").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let output = run_client_command(shared, &["--color", "always", "log"])?;

Expand All @@ -122,7 +123,7 @@ async fn json() -> Result<()> {

// Add a task and wait until it finishes.
assert_success(add_task(shared, "echo test").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let output = run_client_command(shared, &["log", "--json"])?;

Expand Down
22 changes: 11 additions & 11 deletions pueue/tests/client/integration/restart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::HashMap;
use anyhow::Result;
use assert_matches::assert_matches;

use pueue_lib::task::{TaskResult, TaskStatus};
use pueue_lib::task::{Task, TaskResult, TaskStatus};

use crate::client::helper::*;

Expand All @@ -15,7 +15,7 @@ async fn restart_and_edit_task_command() -> Result<()> {

// Create a task and wait for it to finish.
assert_success(add_task(shared, "ls").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

// Set the editor to a command which replaces the temporary file's content.
let mut envs = HashMap::new();
Expand All @@ -26,7 +26,7 @@ async fn restart_and_edit_task_command() -> Result<()> {

// Restart the command, edit its command and wait for it to start.
run_client_command_with_env(shared, &["restart", "--in-place", "--edit", "0"], envs)?;
wait_for_task_condition(shared, 0, |task| task.is_running()).await?;
wait_for_task_condition(shared, 0, Task::is_running).await?;

// Make sure that both the command has been updated.
let state = get_state(shared).await?;
Expand All @@ -49,15 +49,15 @@ async fn restart_and_edit_task_path() -> Result<()> {

// Create a task and wait for it to finish.
assert_success(add_task(shared, "ls").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

// Set the editor to a command which replaces the temporary file's content.
let mut envs = HashMap::new();
envs.insert("EDITOR", "echo '/tmp' > ${PUEUE_EDIT_PATH}/0/path ||");

// Restart the command, edit its command and wait for it to start.
run_client_command_with_env(shared, &["restart", "--in-place", "--edit", "0"], envs)?;
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

// Make sure that both the path has been updated.
let state = get_state(shared).await?;
Expand All @@ -75,7 +75,7 @@ async fn restart_and_edit_task_path_and_command() -> Result<()> {

// Create a task and wait for it to finish.
assert_success(add_task(shared, "ls").await.unwrap());
wait_for_task_condition(shared, 0, |task| task.is_done())
wait_for_task_condition(shared, 0, Task::is_done)
.await
.unwrap();

Expand All @@ -92,7 +92,7 @@ echo '5' > ${PUEUE_EDIT_PATH}/0/priority || ",
// Restart the command, edit its command and path and wait for it to start.
// The task will fail afterwards, but it should still be edited.
run_client_command_with_env(shared, &["restart", "--in-place", "--edit", "0"], envs)?;
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

// Make sure that both the path has been updated.
let state = get_state(shared).await?;
Expand Down Expand Up @@ -123,15 +123,15 @@ async fn restart_and_edit_task_priority() -> Result<()> {

// Create a task and wait for it to finish.
assert_success(add_task(shared, "ls").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

// Set the editor to a command which replaces the temporary file's content.
let mut envs = HashMap::new();
envs.insert("EDITOR", "echo '99' > ${PUEUE_EDIT_PATH}/0/priority ||");

// Restart the command, edit its priority and wait for it to start.
run_client_command_with_env(shared, &["restart", "--in-place", "--edit", "0"], envs)?;
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

// Make sure that the priority has been updated.
let state = get_state(shared).await?;
Expand All @@ -149,7 +149,7 @@ async fn normal_restart_with_edit() -> Result<()> {

// Create a task and wait for it to finish.
assert_success(add_task(shared, "ls").await?);
let original_task = wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
let original_task = wait_for_task_condition(shared, 0, Task::is_done).await?;

// Set the editor to a command which replaces the temporary file's content.
let mut envs = HashMap::new();
Expand All @@ -160,7 +160,7 @@ async fn normal_restart_with_edit() -> Result<()> {

// Restart the command, edit its command and wait for it to start.
run_client_command_with_env(shared, &["restart", "--edit", "0"], envs)?;
wait_for_task_condition(shared, 1, |task| task.is_running()).await?;
wait_for_task_condition(shared, 1, Task::is_running).await?;

// Make sure that both the command has been updated.
let state = get_state(shared).await?;
Expand Down
14 changes: 7 additions & 7 deletions pueue/tests/client/integration/status.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use anyhow::Context;
use anyhow::Result;
use pueue_lib::state::State;
use anyhow::{Context, Result};

use pueue_lib::{state::State, task::Task};

use crate::client::helper::*;

Expand Down Expand Up @@ -42,7 +42,7 @@ async fn full() -> Result<()> {
//
// // Add a task and wait until it finishes.
// assert_success(add_task(shared, "ls").await?);
// wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
// wait_for_task_condition(shared, 0, Task::is_done).await?;
//
// let output = run_status_without_path(shared, &["--color", "always"]).await?;
//
Expand All @@ -67,7 +67,7 @@ async fn single_group() -> Result<()> {
run_client_command(shared, &["add", "--stashed", "ls"])?;

// Make sure the first task finished.
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let output = run_status_without_path(shared, &["--group", "testgroup"]).await?;

Expand All @@ -94,7 +94,7 @@ async fn multiple_groups() -> Result<()> {
run_client_command(shared, &["add", "--group", "testgroup2", "ls"])?;

// Make sure the second task finished.
wait_for_task_condition(shared, 1, |task| task.is_done()).await?;
wait_for_task_condition(shared, 1, Task::is_done).await?;

let output = run_status_without_path(shared, &[]).await?;

Expand All @@ -113,7 +113,7 @@ async fn json() -> Result<()> {

// Add a task and wait until it finishes.
assert_success(add_task(shared, "ls").await?);
wait_for_task_condition(shared, 0, |task| task.is_done()).await?;
wait_for_task_condition(shared, 0, Task::is_done).await?;

let output = run_client_command(shared, &["status", "--json"])?;

Expand Down
Loading

0 comments on commit d86b9c8

Please sign in to comment.