Skip to content

stdlib: Add back recursive lookup for tests#8632

Merged
fdncred merged 1 commit intonushell:mainfrom
presidento:std-tests-glob
Mar 28, 2023
Merged

stdlib: Add back recursive lookup for tests#8632
fdncred merged 1 commit intonushell:mainfrom
presidento:std-tests-glob

Conversation

@presidento
Copy link
Copy Markdown
Contributor

@amtoine during the refactor the test runner lost the ability for looking up for test modules in subdirectories. I just added it back now.

@fdncred fdncred added the A:std-library Defining and improving the standard library written in Nu label Mar 26, 2023
Copy link
Copy Markdown
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot, i might have been a bit too quick on this during the refactoring 😉

only a very minor thing and i'll approve for sure 🥳

Copy link
Copy Markdown
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code looks good to me thanks for the small change 😌

i do not approve, the time i test this locally 😉

@amtoine
Copy link
Copy Markdown
Member

amtoine commented Mar 28, 2023

i have a strange error on machine...
from the tip of your branch

>_ cargo run -- crates/nu-utils/standard_library/tests.nu
Error: nu::parser::parse_mismatch

  × Parse mismatch during operation.
    ╭─[/home/amtoine/.local/share/git/store/github.com/amtoine/nushell/crates/nu-utils/standard_library/test_asserts.nu:13:1]
 13 │     assert error { assert equal 1 "foo" }
 14 │     assert error { assert equal (1 + 2) 4) }
    ·                                         ─┬
    ·                                          ╰── expected number-like value (int, float, date, etc)
 15 │ }
    ╰────

does this cargo run call work for you @presidento ? 😮

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Mar 28, 2023

The problem is that these two lines need to look like this

    assert error { assert equal (1 + 2) "4)" }

and

    assert not equal (1 + 2) "3)"

For some reason they were just 4) and 3)

Copy link
Copy Markdown
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooookey, found it thanks to @fdncred 👌

i've introduced a very bad set of tests in #8150 and we see them in action here...

we need to apply the following patch

diff --git a/crates/nu-utils/standard_library/test_asserts.nu b/crates/nu-utils/standard_library/test_asserts.nu
index ddb21049f..1c1dc30c8 100644
--- a/crates/nu-utils/standard_library/test_asserts.nu
+++ b/crates/nu-utils/standard_library/test_asserts.nu
@@ -11,13 +11,13 @@ export def test_assert_equal [] {
     assert equal (1 + 2) 3
     assert equal (0.1 + 0.2 | into string | into decimal) 0.3 # 0.30000000000000004 == 0.3
     assert error { assert equal 1 "foo" }
-    assert error { assert equal (1 + 2) 4) }
+    assert error { assert equal (1 + 2) 4 }
 }
 
 export def test_assert_not_equal [] {
     assert not equal (1 + 2) 4
     assert not equal 1 "foo"
-    assert not equal (1 + 2) 3)
+    assert not equal (1 + 2) "3"
     assert error { assert not equal 1 1 }
 }

and the tests pass 👍

i think we can land this PR and i'll patch the tests at the same time 😌

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Mar 28, 2023

ok, thanks.

@fdncred fdncred merged commit d391e91 into nushell:main Mar 28, 2023
fdncred pushed a commit that referenced this pull request Mar 28, 2023
Related to #8150, #8635 and #8632.

# Description
i've introduced a bad set of tests for the `assert equal` command in
#8150...

they should not compare `1 + 2` and `4)` or `3)` but the ints.

in this PR, i remove this spurious parentheses that were not planned at
all 😬 👀


# User-Facing Changes
```
$nothing
```

# Tests + Formatting
```
>_ nu crates/nu-utils/standard_library/tests.nu
INF|2023-03-28T20:18:13.022|Running tests in test_asserts
INF|2023-03-28T20:18:13.173|Running tests in test_dirs
INF|2023-03-28T20:18:13.247|Running tests in test_logger
INF|2023-03-28T20:18:13.473|Running tests in test_std
```

# After Submitting
```
$nothing
```
WindSoilder pushed a commit to WindSoilder/nushell that referenced this pull request Mar 29, 2023
@amtoine during the refactor the test runner lost the ability for
looking up for test modules in subdirectories. I just added it back now.
WindSoilder pushed a commit to WindSoilder/nushell that referenced this pull request Mar 29, 2023
Related to nushell#8150, nushell#8635 and nushell#8632.

# Description
i've introduced a bad set of tests for the `assert equal` command in
nushell#8150...

they should not compare `1 + 2` and `4)` or `3)` but the ints.

in this PR, i remove this spurious parentheses that were not planned at
all 😬 👀


# User-Facing Changes
```
$nothing
```

# Tests + Formatting
```
>_ nu crates/nu-utils/standard_library/tests.nu
INF|2023-03-28T20:18:13.022|Running tests in test_asserts
INF|2023-03-28T20:18:13.173|Running tests in test_dirs
INF|2023-03-28T20:18:13.247|Running tests in test_logger
INF|2023-03-28T20:18:13.473|Running tests in test_std
```

# After Submitting
```
$nothing
```
@presidento presidento deleted the std-tests-glob branch April 6, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:std-library Defining and improving the standard library written in Nu

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants