Add comprehensive tests to mq-check by harehare · Pull Request #1420 · harehare/mq

harehare · 2026-03-11T08:45:30Z

Added unit tests and parameterized tests (using rstest) to constraint.rs, lib.rs, main.rs, and types.rs in the mq-check crate to improve test coverage and ensure correctness.

PR created automatically by Jules for task 16570168161836480737 started by @harehare

Added unit tests and parameterized tests (using rstest) to: - src/types.rs: tests for type union, subtraction, matching, and more. - src/lib.rs: tests for TypeEnv, TypeError, and walk_ancestors. - src/constraint.rs: tests for HIR indexing and various helper functions. - src/main.rs: tests for CLI argument parsing. This improves test coverage and helps ensure the correctness of the type checker. Co-authored-by: harehare <[email protected]>

google-labs-jules · 2026-03-11T08:45:31Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

Copilot

Pull request overview

This PR increases test coverage in the mq-check crate by adding new unit tests and parameterized tests (via rstest) across core type-checking modules to validate CLI parsing, type utilities, and HIR/constraint helpers.

Changes:

Added rstest-based parameterized tests for various type operations in types.rs.
Added unit tests for TypeEnv, error location extraction, and ancestor walking in lib.rs.
Added unit tests for constraint-generation helpers (children index, attribute typing, quote/pipe behavior) in constraint.rs.
Added CLI argument parsing unit tests in main.rs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
crates/mq-check/src/types.rs	Adds parameterized tests for union/subtract/matching and substitution/type scheme behavior.
crates/mq-check/src/main.rs	Adds unit tests for clap CLI parsing and flags.
crates/mq-check/src/lib.rs	Adds tests for `TypeEnv`, `TypeError::location`, and `walk_ancestors`.
crates/mq-check/src/constraint.rs	Adds tests for constraint-related helper functions (children index, pipe input, quote blocks, etc.).

Copilot · 2026-03-11T08:49:47Z

crates/mq-check/src/types.rs

+    #[rstest]
+    #[case(vec![Type::Number, Type::Number], Type::Number)]
+    #[case(vec![Type::Number, Type::String], Type::union(vec![Type::Number, Type::String]))]
+    #[case(vec![Type::union(vec![Type::Number, Type::String]), Type::Bool], Type::union(vec![Type::Number, Type::String, Type::Bool]))]
+    #[case(vec![Type::Number, Type::String, Type::Number], Type::union(vec![Type::Number, Type::String]))]
+    fn test_type_union(#[case] types: Vec<Type>, #[case] expected: Type) {
+        assert_eq!(Type::union(types), expected);
+    }
+
+    #[rstest]
+    #[case(Type::union(vec![Type::Number, Type::String]), &Type::Number, Type::String)]
+    #[case(Type::union(vec![Type::Number, Type::String, Type::Bool]), &Type::String, Type::union(vec![Type::Number, Type::Bool]))]
+    #[case(Type::Number, &Type::Number, Type::Number)]
+    fn test_type_subtract(#[case] ty: Type, #[case] exclude: &Type, #[case] expected: Type) {
+        assert_eq!(ty.subtract(exclude), expected);


test_type_union builds the expected value using Type::union(...) in multiple cases. That makes those assertions largely tautological (if Type::union is wrong, the expected is computed the same wrong way) and weakens the regression value of the test. Prefer constructing the expected value without calling the function under test (e.g., explicitly matching on Type::Union members / ordering, or asserting properties like member count and presence).

Suggested change

#[rstest]

#[case(vec![Type::Number, Type::Number], Type::Number)]

#[case(vec![Type::Number, Type::String], Type::union(vec![Type::Number, Type::String]))]

#[case(vec![Type::union(vec![Type::Number, Type::String]), Type::Bool], Type::union(vec![Type::Number, Type::String, Type::Bool]))]

#[case(vec![Type::Number, Type::String, Type::Number], Type::union(vec![Type::Number, Type::String]))]

fn test_type_union(#[case] types: Vec<Type>, #[case] expected: Type) {

assert_eq!(Type::union(types), expected);

}

#[rstest]

#[case(Type::union(vec![Type::Number, Type::String]), &Type::Number, Type::String)]

#[case(Type::union(vec![Type::Number, Type::String, Type::Bool]), &Type::String, Type::union(vec![Type::Number, Type::Bool]))]

#[case(Type::Number, &Type::Number, Type::Number)]

fn test_type_subtract(#[case] ty: Type, #[case] exclude: &Type, #[case] expected: Type) {

assert_eq!(ty.subtract(exclude), expected);

#[test]

fn test_type_union() {

// Union of identical types collapses to a single type

let ty = Type::union(vec![Type::Number, Type::Number]);

assert_eq!(ty, Type::Number);

// Union of two distinct types produces a union containing both

let ty = Type::union(vec![Type::Number, Type::String]);

match ty {

Type::Union(members) => {

assert_eq!(members.len(), 2);

assert!(members.contains(&Type::Number));

assert!(members.contains(&Type::String));

}

other => panic!("expected Type::Union for Number | String, got {:?}", other),

}

// Union of an existing union with another type flattens and includes all members

let ty = Type::union(vec![

Type::union(vec![Type::Number, Type::String]),

Type::Bool,

]);

match ty {

Type::Union(members) => {

assert_eq!(members.len(), 3);

assert!(members.contains(&Type::Number));

assert!(members.contains(&Type::String));

assert!(members.contains(&Type::Bool));

}

other => panic!(

"expected Type::Union for (Number | String) | Bool, got {:?}",

other

),

}

// Duplicates are removed when building unions

let ty = Type::union(vec![Type::Number, Type::String, Type::Number]);

match ty {

Type::Union(members) => {

assert_eq!(members.len(), 2);

assert!(members.contains(&Type::Number));

assert!(members.contains(&Type::String));

}

other => panic!(

"expected Type::Union with Number and String, got {:?}",

other

),

}

}

#[test]

fn test_type_subtract() {

// Removing a member from a union leaves the remaining member

let ty = Type::union(vec![Type::Number, Type::String]);

let result = ty.subtract(&Type::Number);

assert_eq!(result, Type::String);

// Removing a member from a larger union removes only that member

let ty = Type::union(vec![Type::Number, Type::String, Type::Bool]);

let result = ty.subtract(&Type::String);

match result {

Type::Union(members) => {

assert_eq!(members.len(), 2);

assert!(members.contains(&Type::Number));

assert!(members.contains(&Type::Bool));

assert!(!members.contains(&Type::String));

}

other => panic!(

"expected Type::Union with Number and Bool after subtracting String, got {:?}",

other

),

}

// Subtracting the same type from a non-union leaves it unchanged (idempotent)

let ty = Type::Number;

let result = ty.subtract(&Type::Number);

assert_eq!(result, Type::Number);

Copilot · 2026-03-11T08:49:48Z

crates/mq-check/src/constraint.rs

+        // Verify that the setup created a quote keyword parent
+        let has_quote_parent = walk_ancestors(&hir, x_ref)
+            .any(|(_, s)| matches!(s.kind, SymbolKind::Keyword) && s.value.is_none());
+
+        if has_quote_parent {
+            assert!(is_inside_quote_block(&hir, x_ref));
+        }


test_is_inside_quote_block is currently conditional: if the quote keyword ancestor isn't found, the test performs no assertion and will pass even if is_inside_quote_block is broken or the input no longer produces a quote ancestor. Make this test assert that the quote ancestor exists (or use an input that guarantees it), then assert is_inside_quote_block returns true.

Suggested change

// Verify that the setup created a quote keyword parent

let has_quote_parent = walk_ancestors(&hir, x_ref)

.any(|(_, s)| matches!(s.kind, SymbolKind::Keyword) && s.value.is_none());

if has_quote_parent {

assert!(is_inside_quote_block(&hir, x_ref));

}

// Verify that the setup created a quote keyword parent; this must hold

let has_quote_parent = walk_ancestors(&hir, x_ref)

.any(|(_, s)| matches!(s.kind, SymbolKind::Keyword) && s.value.is_none());

assert!(

has_quote_parent,

"Expected 'x' to have a quote keyword ancestor in HIR"

);

assert!(is_inside_quote_block(&hir, x_ref));

Added unit tests and parameterized tests (using rstest) to: - src/types.rs: tests for type union, subtraction, matching, and more. - src/lib.rs: tests for TypeEnv, TypeError, and walk_ancestors. - src/constraint.rs: tests for HIR indexing and various helper functions. - src/main.rs: tests for CLI argument parsing. This improves test coverage and helps ensure the correctness of the type checker. Ran cargo fmt to ensure code style consistency. Co-authored-by: harehare <[email protected]>

Copilot AI review requested due to automatic review settings March 11, 2026 08:45

Copilot started reviewing on behalf of harehare March 11, 2026 08:45 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

harehare merged commit 605171a into main Mar 11, 2026
4 checks passed

harehare deleted the add-comprehensive-tests-mq-check-16570168161836480737 branch March 11, 2026 10:51

BrewTestBot mentioned this pull request Mar 11, 2026

mq 0.5.18 Homebrew/homebrew-core#271763

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add comprehensive tests to mq-check#1420

Add comprehensive tests to mq-check#1420
harehare merged 2 commits intomainfrom
add-comprehensive-tests-mq-check-16570168161836480737

harehare commented Mar 11, 2026

Uh oh!

google-labs-jules bot commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    #[rstest]
-    #[case(vec![Type::Number, Type::Number], Type::Number)]
-    #[case(vec![Type::Number, Type::String], Type::union(vec![Type::Number, Type::String]))]
-    #[case(vec![Type::union(vec![Type::Number, Type::String]), Type::Bool], Type::union(vec![Type::Number, Type::String, Type::Bool]))]
-    #[case(vec![Type::Number, Type::String, Type::Number], Type::union(vec![Type::Number, Type::String]))]
-    fn test_type_union(#[case] types: Vec<Type>, #[case] expected: Type) {
-        assert_eq!(Type::union(types), expected);
-    }
-    #[rstest]
-    #[case(Type::union(vec![Type::Number, Type::String]), &Type::Number, Type::String)]
-    #[case(Type::union(vec![Type::Number, Type::String, Type::Bool]), &Type::String, Type::union(vec![Type::Number, Type::Bool]))]
-    #[case(Type::Number, &Type::Number, Type::Number)]
-    fn test_type_subtract(#[case] ty: Type, #[case] exclude: &Type, #[case] expected: Type) {
-        assert_eq!(ty.subtract(exclude), expected);
+    #[test]
+    fn test_type_union() {
+        // Union of identical types collapses to a single type
+        let ty = Type::union(vec![Type::Number, Type::Number]);
+        assert_eq!(ty, Type::Number);
+        // Union of two distinct types produces a union containing both
+        let ty = Type::union(vec![Type::Number, Type::String]);
+        match ty {
+            Type::Union(members) => {
+                assert_eq!(members.len(), 2);
+                assert!(members.contains(&Type::Number));
+                assert!(members.contains(&Type::String));
+            }
+            other => panic!("expected Type::Union for Number | String, got {:?}", other),
+        }
+        // Union of an existing union with another type flattens and includes all members
+        let ty = Type::union(vec![
+            Type::union(vec![Type::Number, Type::String]),
+            Type::Bool,
+        ]);
+        match ty {
+            Type::Union(members) => {
+                assert_eq!(members.len(), 3);
+                assert!(members.contains(&Type::Number));
+                assert!(members.contains(&Type::String));
+                assert!(members.contains(&Type::Bool));
+            }
+            other => panic!(
+                "expected Type::Union for (Number | String) | Bool, got {:?}",
+                other
+            ),
+        }
+        // Duplicates are removed when building unions
+        let ty = Type::union(vec![Type::Number, Type::String, Type::Number]);
+        match ty {
+            Type::Union(members) => {
+                assert_eq!(members.len(), 2);
+                assert!(members.contains(&Type::Number));
+                assert!(members.contains(&Type::String));
+            }
+            other => panic!(
+                "expected Type::Union with Number and String, got {:?}",
+                other
+            ),
+        }
+    }
+    #[test]
+    fn test_type_subtract() {
+        // Removing a member from a union leaves the remaining member
+        let ty = Type::union(vec![Type::Number, Type::String]);
+        let result = ty.subtract(&Type::Number);
+        assert_eq!(result, Type::String);
+        // Removing a member from a larger union removes only that member
+        let ty = Type::union(vec![Type::Number, Type::String, Type::Bool]);
+        let result = ty.subtract(&Type::String);
+        match result {
+            Type::Union(members) => {
+                assert_eq!(members.len(), 2);
+                assert!(members.contains(&Type::Number));
+                assert!(members.contains(&Type::Bool));
+                assert!(!members.contains(&Type::String));
+            }
+            other => panic!(
+                "expected Type::Union with Number and Bool after subtracting String, got {:?}",
+                other
+            ),
+        }
+        // Subtracting the same type from a non-union leaves it unchanged (idempotent)
+        let ty = Type::Number;
+        let result = ty.subtract(&Type::Number);
+        assert_eq!(result, Type::Number);

Conversation

harehare commented Mar 11, 2026

Uh oh!

google-labs-jules bot commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants