Add more tests

[kdheepak]

Related #83

[kdheepak]

Hi @orhun, I think I'd like to change the tests to include the --test-threads=1 flag. Since I'm running taskwarrior as a subprocess, when tests are run using multiple threads it fails randomly.

I'm assuming you'll have to update this line https://github.com/archlinux/svntogit-community/blob/5a37f121adf34fc129994f7dad474093edbad9a9/taskwarrior-tui/trunk/PKGBUILD#L27 to the following:

cargo test --release --locked --all-features -- --test-threads=1

I'm also open to suggestions on how to handle this better. I think you've looked at more packages and how they all run tests, so I definitely value any suggestions you may have.

[kdheepak]

@orhun I'll check in with you again before making a release. I'll continue using --test-threads=1 for now.

[orhun]

Hi @kdheepak!

Since I'm running taskwarrior as a subprocess, when tests are run using multiple threads it fails randomly.

I see. Which tests are failing occasionally when you run them asynchronously via cargo test? I'd guess that TUI events may be clashing on runtime. I'd convert the unit tests into integration tests in that case. Like merging some tests into one module to run them in order etc.

I might actually work on that and submit a PR if you consider to do that.

I'd like to change the tests to include the --test-threads=1 flag.

I guess it's totally fine as long as you document it :)

I'm assuming you'll have to update this line https://github.com/archlinux/svntogit-community/blob/5a37f121adf34fc129994f7dad474093edbad9a9/taskwarrior-tui/trunk/PKGBUILD#L27:

That's correct.

Thanks for poking me about this, although I couldn't respond quickly enough :/

[kdheepak]

Thanks for poking me about this, although I couldn't respond quickly enough :/

No worries! I didn't think it was time critical since I wasn't planning to make a release right now.

I might actually work on that and submit a PR if you consider to do that.

I don't want to give you more things to do! But if you submit a PR I'd be more than happy to consider it.

Which tests are failing occasionally when you run them asynchronously via cargo test?

Currently, I have this test which checks that the context switching works correctly.

taskwarrior-tui/src/app.rs

Lines 1789 to 1823 in 067fe44

	#[test]
	fn test_task_context() {
	let mut app = TTApp::new().unwrap();
	assert!(app.get_context().is_ok());
	assert!(app.update().is_ok());
	app.context_next();
	app.context_select();
	assert_eq!(app.tasks.lock().unwrap().len(), 26);
	assert_eq!(app.current_context_filter, "");
	assert_eq!(app.context_table_state.selected(), Some(0));
	app.context_next();
	app.context_select();
	assert_eq!(app.context_table_state.selected(), Some(1));
	assert!(app.get_context().is_ok());
	assert!(app.update().is_ok());
	assert_eq!(app.tasks.lock().unwrap().len(), 1);
	assert_eq!(app.current_context_filter, "+finance -private");
	assert_eq!(app.context_table_state.selected(), Some(1));
	app.context_previous();
	app.context_select();
	assert_eq!(app.context_table_state.selected(), Some(0));
	assert!(app.get_context().is_ok());
	assert!(app.update().is_ok());
	assert_eq!(app.tasks.lock().unwrap().len(), 26);
	assert_eq!(app.current_context_filter, "");
	}

and this test

taskwarrior-tui/src/app.rs

Lines 1825 to 1890 in 067fe44

	#[test]
	fn test_task_tomorrow() {
	let total_tasks: u64 = 26;
	let mut app = TTApp::new().unwrap();
	assert!(app.get_context().is_ok());
	assert!(app.update().is_ok());
	assert_eq!(app.tasks.lock().unwrap().len(), total_tasks as usize);
	assert_eq!(app.current_context_filter, "");
	let now = Local::now();
	let now = TimeZone::from_utc_datetime(now.offset(), &now.naive_utc());
	let mut command = Command::new("task");
	command.arg("add");
	let message = format!(
	"'new task for testing tomorrow' due:{:04}-{:02}-{:02}",
	now.year(),
	now.month(),
	now.day() + 1
	);
	let shell = message.as_str().replace("'", "\\'");
	let cmd = shlex::split(&shell).unwrap();
	for s in cmd {
	command.arg(&s);
	}
	let output = command.output().unwrap();
	let s = String::from_utf8_lossy(&output.stdout);
	let re = Regex::new(r"^Created task (?P<task_id>\d+).\n$").unwrap();
	let caps = re.captures(&s).unwrap();
	let task_id = caps["task_id"].parse::<u64>().unwrap();
	assert_eq!(task_id, total_tasks + 1);
	assert!(app.get_context().is_ok());
	assert!(app.update().is_ok());
	assert_eq!(app.tasks.lock().unwrap().len(), (total_tasks + 1) as usize);
	assert_eq!(app.current_context_filter, "");
	let task = app.task_by_id(task_id).unwrap();
	for s in &[
	"DUE",
	"MONTH",
	"PENDING",
	"QUARTER",
	"TOMORROW",
	"UDA",
	"UNBLOCKED",
	"YEAR",
	] {
	assert!(task.tags().unwrap().contains(&s.to_string()));
	}
	let output = Command::new("task")
	.arg("rc.confirmation=off")
	.arg("undo")
	.output()
	.unwrap();
	let mut app = TTApp::new().unwrap();
	assert!(app.get_context().is_ok());
	assert!(app.update().is_ok());
	assert_eq!(app.tasks.lock().unwrap().len(), total_tasks as usize);
	assert_eq!(app.current_context_filter, "");
	}

which checks the number of tasks before and after adding a task and undoing the previous action.

When the context changes the number of tasks changes and the tests fail.

Currently for the TUI tests, this is what I'm doing.

taskwarrior-tui/src/app.rs

Lines 1961 to 2048 in 067fe44

	#[test]
	fn test_draw_task_report() {
	let test_case = |expected: &Buffer| {
	let mut app = TTApp::new().unwrap();
	app.task_report_next();
	app.context_next();
	let backend = TestBackend::new(50, 15);
	let mut terminal = Terminal::new(backend).unwrap();
	terminal
	.draw(|f| {
	app.draw(f);
	app.draw(f);
	})
	.unwrap();
	assert_eq!(terminal.backend().size().unwrap(), expected.area);
	terminal.backend().assert_buffer(expected);
	};
	let mut expected = Buffer::with_lines(vec![
	"╭Task|Calendar───────────────────────────────────╮",
	"│ ID Age Deps P Proj Tag Due Until Descr Urg │",
	"│ │",
	"│• 8 4mo U Run … 23.00│",
	"│ 10 4mo colo COLOR -8d … [2] 14.80│",
	"╰────────────────────────────────────────────────╯",
	"╭Task 8──────────────────────────────────────────╮",
	"│ │",
	"│Name Value │",
	"│----------- ------------------------------------│",
	"│ID 8 │",
	"╰────────────────────────────────────────────────╯",
	"╭Filter Tasks────────────────────────────────────╮",
	"│status:pending -private │",
	"╰────────────────────────────────────────────────╯",
	]);
	for i in 1..=4 {
	// Task
	expected
	.get_mut(i, 0)
	.set_style(Style::default().add_modifier(Modifier::BOLD));
	}
	for i in 6..=13 {
	// Calendar
	expected
	.get_mut(i, 0)
	.set_style(Style::default().add_modifier(Modifier::DIM));
	}
	for r in &[
	1..=4, // ID
	6..=8, // Age
	10..=13, // Deps
	15..=15, // P
	17..=20, // Proj
	22..=26, // Tag
	28..=30, // Due
	32..=36, // Until
	38..=42, // Descr
	44..=48, // Urg
	] {
	for i in r.clone().into_iter() {
	expected
	.get_mut(i, 1)
	.set_style(Style::default().add_modifier(Modifier::UNDERLINED));
	}
	}
	for i in 1..expected.area().width - 1 {
	expected.get_mut(i, 3).set_style(
	Style::default()
	.fg(Color::Indexed(0))
	.bg(Color::Indexed(15))
	.add_modifier(Modifier::BOLD),
	);
	}
	for i in 1..expected.area().width - 1 {
	expected
	.get_mut(i, 4)
	.set_style(Style::default().fg(Color::Indexed(9)).bg(Color::Indexed(4)));
	}
	test_case(&expected);
	}

This doesn't test for events though. For testing events I'll definitely need multi-threading though.

I think one thing I can do is write 1 [#test] function that sequentially calls all the functions that cannot be tests in a multi threaded fashion. If I do that then cargo test will just work.

[kdheepak]

I think one thing I can do is write 1 [#test] function that sequentially calls all the functions that cannot be tests in a multi threaded fashion. If I do that then cargo test will just work.

I've decided to do this:

taskwarrior-tui/src/app.rs

Lines 1789 to 1796 in 13e4cf2

	#[test]
	fn test_taskwarrior_in_order() {
	test_task_context();
	test_task_tomorrow();
	test_task_earlier_today();
	test_task_later_today();
	test_draw_task_report();
	}

This way the standard use of cargo test will work, and I think having that is useful. If I required --test-threads=1 and I accidentally ran cargo test without that flag, the tests would fail and the test data for taskwarrior would be modified. This has also happened multiple times because I'm just used to typing cargo test. I think I'll plan to make sure that cargo test always works. So no need for any change on your end right now! Sorry for the confusion.

[kdheepak]

I've made a new release: https://github.com/kdheepak/taskwarrior-tui/releases/tag/v0.11.0

There shouldn't be any changes on your end, but let me know if something breaks. Running cargo test without any arguments will work.

[orhun]

No problems! 0.11.0 is in the repos right now: https://archlinux.org/packages/community/x86_64/taskwarrior-tui/

add 4748c6a4-8f98-4bb7-8650-a92c971e17e0 Maßnahmen

And oh hey, I learned a new German word! Ich soll das neu Wort aufschreiben, auf jeden Fall nützlich für meinen Deutschkurs!

[kdheepak]

Nice that you are learning German! And your German definitely google translated well for me :) So good job!