Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdlib: the tests do not run using the prelude #8809

Closed
amtoine opened this issue Apr 8, 2023 · 0 comments · Fixed by #8811
Closed

stdlib: the tests do not run using the prelude #8809

amtoine opened this issue Apr 8, 2023 · 0 comments · Fixed by #8811
Labels
enhancement New feature or request std-library Defining and improving the standard library written in nu and the core Rust ccommands tests issues to add tests or fix tests
Milestone

Comments

@amtoine
Copy link
Member

amtoine commented Apr 8, 2023

Related problem

the tests of the standard library uses the modules (relative) paths to import required commands, e.e

use std.nu 'assert equal'

now, we have a prelude loading thanks to #8627, which loads the help commands from #8505.
things can now be imported from anywhere with

use std ...

Note
we do not need the name of the file std.nu but only the "abstract" name of the library, i.e. std[

Describe the solution you'd like

i think we should use

use std ...

in the tests not to rely on the filesystem paths.

we also need to mention in the README that, as the library is now embedded in the nu binary, we should run the tests with

cargo run -- crates/nu-std/tests.nu

to avoid using the installed nu which could be quite out of sync 🤔

Describe alternatives you've considered

No response

Additional context and details

No response

@amtoine amtoine added the enhancement New feature or request label Apr 8, 2023
@sholderbach sholderbach added tests issues to add tests or fix tests std-library Defining and improving the standard library written in nu and the core Rust ccommands labels Apr 8, 2023
fdncred pushed a commit that referenced this issue Apr 8, 2023
Should close #8809.

# Description
this PR uses the automatically loaded library from the tests by
replacing `use std.nu ...` with `use std ...`.

the `README` has been updated by
- removing the very deprencated "concrete examples"
- fixing the `use std` and the "run the tests" sections

the `README` can be previewed
[here](https://github.com/amtoine/nushell/blob/refactor/stdlib/use-std-in-tests-and-update-readme/crates/nu-std/README.md)
:+1:

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

# Tests + Formatting
- :green_circle: `toolkit test stdlib`

# After Submitting
```
$nothing
```
@hustcer hustcer added this to the v0.79.0 milestone Apr 11, 2023
MariaSolOs added a commit to MariaSolOs/nushell that referenced this issue Apr 12, 2023
commit 8ee52b6
Author: JT <547158+jntrnr@users.noreply.github.com>
Date:   Wed Apr 12 05:21:52 2023 +1200

    Relax the closure syntax, highlight differently (nushell#8846)

    This relaxes the closure syntax so that `||` is no longer required. This
    allows for `ls | each { $in.name }` for example.

    I've gone ahead and changed the syntax highlighting so that blocks and
    closures are distinct for now.

    Removes `||` requirement for closures.

    Don't forget to add tests that cover your changes.

    Make sure you've run and fixed any issues with these commands:

    - `cargo fmt --all -- --check` to check standard code formatting (`cargo
    fmt --all` applies these changes)
    - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
    clippy::needless_collect` to check that you're using the standard code
    style
    - `cargo test --workspace` to check that all tests pass
    - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
    standard library

    > **Note**
    > from `nushell` you can also use the `toolkit` as follows
    > ```bash
    > use toolkit.nu # or use an `env_change` hook to activate it
    automatically
    > toolkit check pr
    > ```

    If your PR had any user-facing changes, update [the
    documentation](https://github.com/nushell/nushell.github.io) after the
    PR is merged, if necessary. This will help us keep the docs up to date.

commit 46dba88
Author: JT <547158+jntrnr@users.noreply.github.com>
Date:   Wed Apr 12 05:21:31 2023 +1200

    Revert numberlike parsing restriction (nushell#8845)

    This effectively reverts nushell#8635. We shipped this change with 0.78 and
    received many comments/issues related to this restriction feeling like a
    step backward.

    fixes: nushell#8844
    (and probably other issues)

    Returns numbers and number-like values to being allowed to be bare
    words. Examples: `3*`, `1fb43`, `4,5`, and related.

    Don't forget to add tests that cover your changes.

    Make sure you've run and fixed any issues with these commands:

    - `cargo fmt --all -- --check` to check standard code formatting (`cargo
    fmt --all` applies these changes)
    - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
    clippy::needless_collect` to check that you're using the standard code
    style
    - `cargo test --workspace` to check that all tests pass
    - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
    standard library

    > **Note**
    > from `nushell` you can also use the `toolkit` as follows
    > ```bash
    > use toolkit.nu # or use an `env_change` hook to activate it
    automatically
    > toolkit check pr
    > ```

    If your PR had any user-facing changes, update [the
    documentation](https://github.com/nushell/nushell.github.io) after the
    PR is merged, if necessary. This will help us keep the docs up to date.

commit eb4d19f
Author: federico viscomi <federicoviscomi@gmail.com>
Date:   Mon Apr 10 22:52:51 2023 +0100

    Parser panic for signature with multibyte char for short flag nushell#8821 (nushell#8849)

    Hey I'm a developer and I'm still new to nushell and rust but I would
    like to learn more about both. This is my first PR for this project.

    The intent of my change is to allow to use multibyte utf-8 characters in
    commands short flags.

commit c367876
Author: Máté FARKAS <fm@farkas-mate.hu>
Date:   Mon Apr 10 22:42:11 2023 +0200

    stdlib test runner: implement `setup` and `teardown` commands to unit tests (nushell#8776)

    As in other testing frameworks, the `setup` runs before every test case,
    and the `teardown` after that. A context can be created in `setup`,
    which will be in the `$in` variable in the test cases, and in the
    `teardown`. The `teardown` is called regardless of the test is passed,
    skipped, or failed.

    For example:

    ```nushell
    use std.nu *

    export def setup [] {
        log debug "Setup is running"
        {msg: "This is the context"}
    }

    export def teardown [] {
        log debug $"Teardown is running. Context: ($in)"
    }

    export def test_assert_pass [] {
        log debug $"Assert is running. Context: ($in)"
    }

    export def test_assert_skip [] {
        log debug $"Assert is running. Context: ($in)"
        assert skip
    }

    export def test_assert_fail_skipped_by_default [] {
        log debug $"Assert is running. Context: ($in)"
        assert false
    }
    ```

    ![image](https://user-images.githubusercontent.com/282320/230466359-9908cc72-edbd-4150-9aff-d15fe42c0cc7.png)

    I'll update the documentation.

    ---------

    Co-authored-by: Mate Farkas <Mate.Farkas@oneidentity.com>

commit 9bb2c8f
Author: Justin Ma <hustcer@outlook.com>
Date:   Tue Apr 11 04:10:41 2023 +0800

    Upgrade to nu v0.78 for binaries release workflow (nushell#8840)

    Upgrade to nu v0.78 for binary packages release workflow

    Test Release: https://github.com/hustcer/nu-release/releases/tag/v0.78.2
    Release workflow running output:
    https://github.com/hustcer/nu-release/actions/runs/4656319252/jobs/8239828202

    _(List of all changes that impact the user experience here. This helps
    us keep track of breaking changes.)_

    Don't forget to add tests that cover your changes.

    Make sure you've run and fixed any issues with these commands:

    - `cargo fmt --all -- --check` to check standard code formatting (`cargo
    fmt --all` applies these changes)
    - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
    clippy::needless_collect` to check that you're using the standard code
    style
    - `cargo test --workspace` to check that all tests pass
    - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
    standard library

    > **Note**
    > from `nushell` you can also use the `toolkit` as follows
    > ```bash
    > use toolkit.nu # or use an `env_change` hook to activate it
    automatically
    > toolkit check pr
    > ```

    If your PR had any user-facing changes, update [the
    documentation](https://github.com/nushell/nushell.github.io) after the
    PR is merged, if necessary. This will help us keep the docs up to date.

commit 74dcac3
Author: nibon7 <nibon7@163.com>
Date:   Tue Apr 11 02:52:42 2023 +0800

    Fix process_range on 32-bit platforms (nushell#8842)

    This PR fixes
    `commands::str_::substrings_the_input_and_treats_end_index_as_length_if_blank_end_index_given`
    testcase on 32-bit platform.

    ```
    failures:
    ---- commands::str_::substrings_the_input_and_treats_end_index_as_length_if_blank_end_index_given stdout ----
    === stderr
    thread 'commands::str_::substrings_the_input_and_treats_end_index_as_length_if_blank_end_index_given' panicked at 'assertion failed: `(left == right)`
      left: `"arepa"`,
     right: `"arepas"`', crates/nu-command/tests/commands/str_/mod.rs:363:9
    failures:
        commands::str_::substrings_the_input_and_treats_end_index_as_length_if_blank_end_index_given
    test result: FAILED. 1072 passed; 1 failed; 23 ignored; 0 measured; 0 filtered out; finished in 2.98s
    error: test failed, to rerun pass `-p nu-command --test main`
    ```

    https://gitlab.alpinelinux.org/nibon7/aports/-/jobs/1005935#L3864
    https://gitlab.alpinelinux.org/nibon7/aports/-/jobs/1005931#L3867

    N/A

    Don't forget to add tests that cover your changes.

    Make sure you've run and fixed any issues with these commands:

    - `cargo fmt --all -- --check` to check standard code formatting (`cargo
    fmt --all` applies these changes)
    - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
    clippy::needless_collect` to check that you're using the standard code
    style
    - `cargo test --workspace` to check that all tests pass
    - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
    standard library

    > **Note**
    > from `nushell` you can also use the `toolkit` as follows
    > ```bash
    > use toolkit.nu # or use an `env_change` hook to activate it
    automatically
    > toolkit check pr
    > ```

    If your PR had any user-facing changes, update [the
    documentation](https://github.com/nushell/nushell.github.io) after the
    PR is merged, if necessary. This will help us keep the docs up to date.

commit 6d51e34
Author: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
Date:   Mon Apr 10 20:32:33 2023 +0200

    stdlib: make helper modules available in std (nushell#8841)

    > **Note**
    > waiting for the following to land
    > - nushell#8824 to avoid conflicts,
    i'll add this to `CONTRIBUTING.md` once it's ready 👍

    should close nushell#8839
    this PR moves the `log` submodule of `std` to the top of the call stack,
    making it available in the rest of the library as `log`.
    i've added some comments around the `submodules` list in
    `load_standard_library` to make it clear how it should work.
    `log` and `assert` are now available in the rest of `std`.
    - 🟢 `toolkit fmt`
    - 🟢 `toolkit clippy`
    - ⚫ `toolkit test`
    - 🟢 `toolkit test stdlib`
    ```
    $nothing
    ```

commit de76c7a
Author: WindSoilder <WindSoilder@outlook.com>
Date:   Tue Apr 11 01:23:22 2023 +0800

    Remove autoprinting of `for` loop (nushell#8843)

    It's an addition to nushell#8618
    And I think it's good to keep the same behavior when we use for loop for
    list.

    Don't forget to add tests that cover your changes.

    Make sure you've run and fixed any issues with these commands:

    - `cargo fmt --all -- --check` to check standard code formatting (`cargo
    fmt --all` applies these changes)
    - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
    clippy::needless_collect` to check that you're using the standard code
    style
    - `cargo test --workspace` to check that all tests pass
    - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
    standard library

    > **Note**
    > from `nushell` you can also use the `toolkit` as follows
    > ```bash
    > use toolkit.nu # or use an `env_change` hook to activate it
    automatically
    > toolkit check pr
    > ```

    If your PR had any user-facing changes, update [the
    documentation](https://github.com/nushell/nushell.github.io) after the
    PR is merged, if necessary. This will help us keep the docs up to date.

commit 0e23400
Author: Stefan Holderbach <sholderbach@users.noreply.github.com>
Date:   Mon Apr 10 03:42:43 2023 +0200

    Follow up nushell#8758 with a style fix (nushell#8822)

    The old comment around the question mark operator doesn't make sense
     to me based on the closure signature.
    The `match` expressions were thus superfluous.

    None

    No change

commit d0a83fe
Author: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
Date:   Mon Apr 10 00:59:57 2023 +0200

    stdlib: add completion to `help` commands (nushell#8799)

    > **Note**
    > waiting for
    > - nushell#8770
    > - nushell#8815
    this PR adds a very basic completion support to the `help`
    implementations of the `std`.
    completion on all the `std help ...` commands, based on the names in
    `$nu.scope.`
    - ⚫ `toolkit fmt`
    - ⚫ `toolkit clippy`
    - ⚫ `toolkit test`
    - ⚫ `toolkit test stdlib`
    ```
    $nothing
    ```

commit 57510f2
Author: Stefan Holderbach <sholderbach@users.noreply.github.com>
Date:   Mon Apr 10 00:56:47 2023 +0200

    Move CLI related commands to `nu-cli` (nushell#8832)

    Part of the larger cratification effort.

    Moves all `reedline` or shell line editor specific commands to `nu-cli`.
    - `commandline`
    - This shouldn't have moved there. Doesn't directly depend on reedline
    but assumes parts in the engine state that are specific to the use of
    reedline or a REPL
    - `keybindings` and subcommands
      - `keybindings default`
      - `keybindings list`
      - `keybindings listen`
        - very `reedline` specific
    - `history`
      - needs `reedline`
    - `history session`
    Instead of having a separate `create_default_context()` that calls
    `nu-command`'s `create_default_context()`, I added a `add_cli_context()`
    that updates an `EngineState`

    None

    `cargo build --timings` from a `cargo clean --profile dev`
    main: 64 secs
    this: 59 secs

    branch | total| codegen | fraction
    ---|---|---|---
    main | 14.0s | 6.2s | (44%)
    this | 12.5s | 5.5s | (44%)

    `nu-cli` depends on `nu-command` at the moment.
    Thus it is built during the code-gen phase of `nu-command` (on 16
    virtual cores)

    I removed the `test_example()` facilities for now as we had not run any
    of the commands in an `Example` test and importing the right context for
    those tests seemed more of a hassle than the duplicated
    `test_examples()` implementations in `nu-cmd-lang` and `nu-command`

commit 58b96fd
Author: JT <547158+jntrnr@users.noreply.github.com>
Date:   Mon Apr 10 10:55:29 2023 +1200

    Add option to not load std-lib. Default tests to not use std-lib (nushell#8833)

    this adds a `--no-std-lib` flag. Moves `nu!` to use the `--no-std-lib`.
    Adds a new `nu_with_std!` macro for future tests that need the std-lib.

    _(Thank you for improving Nushell. Please, check our [contributing
    guide](../CONTRIBUTING.md) and talk to the core team before making major
    changes.)_

    _(Description of your pull request goes here. **Provide examples and/or
    screenshots** if your changes affect the user experience.)_

    _(List of all changes that impact the user experience here. This helps
    us keep track of breaking changes.)_

    Don't forget to add tests that cover your changes.

    Make sure you've run and fixed any issues with these commands:

    - `cargo fmt --all -- --check` to check standard code formatting (`cargo
    fmt --all` applies these changes)
    - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
    clippy::needless_collect` to check that you're using the standard code
    style
    - `cargo test --workspace` to check that all tests pass
    - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
    standard library

    > **Note**
    > from `nushell` you can also use the `toolkit` as follows
    > ```bash
    > use toolkit.nu # or use an `env_change` hook to activate it
    automatically
    > toolkit check pr
    > ```

    If your PR had any user-facing changes, update [the
    documentation](https://github.com/nushell/nushell.github.io) after the
    PR is merged, if necessary. This will help us keep the docs up to date.

commit 9e3d6c3
Author: JT <547158+jntrnr@users.noreply.github.com>
Date:   Mon Apr 10 08:55:47 2023 +1200

    Only add the std lib files once (nushell#8830)

    We were seeing duplicate entries for the std lib files, and this PR
    addresses that. Each file should now only be added once.

    Note: they are still parsed twice because it's hard to recover the
    module from the output of `parse` but a bit of clever hacking in a
    future PR might be able to do that.

    _(List of all changes that impact the user experience here. This helps
    us keep track of breaking changes.)_

    Don't forget to add tests that cover your changes.

    Make sure you've run and fixed any issues with these commands:

    - `cargo fmt --all -- --check` to check standard code formatting (`cargo
    fmt --all` applies these changes)
    - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
    clippy::needless_collect` to check that you're using the standard code
    style
    - `cargo test --workspace` to check that all tests pass
    - `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
    standard library

    > **Note**
    > from `nushell` you can also use the `toolkit` as follows
    > ```bash
    > use toolkit.nu # or use an `env_change` hook to activate it
    automatically
    > toolkit check pr
    > ```

    If your PR had any user-facing changes, update [the
    documentation](https://github.com/nushell/nushell.github.io) after the
    PR is merged, if necessary. This will help us keep the docs up to date.

commit 4a955d7
Author: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
Date:   Sun Apr 9 19:00:20 2023 +0200

    stdlib: refactor into a multi-module library (nushell#8815)

commit 637283f
Author: mike <98623181+1Kinoti@users.noreply.github.com>
Date:   Sat Apr 8 23:04:57 2023 +0300

    allow register to accept a const argument (nushell#8758)

    this pr allows `register` to be used with const variables
    ```nu
    const math_plugin = "~/.config/nushell/plugins/nu_plugin_math"
    register $math_plugin
    ```

    should close nushell#8208, previous work nushell#8435

commit 5afbfb5
Author: Maria José Solano <majosolano99@gmail.com>
Date:   Sat Apr 8 11:52:37 2023 -0700

    Use `pretty_assertions` in the root crate (nushell#8818)

    This PR is just a minor development improvement. While working on
    another feature, I noticed that the root crate lists the super useful
    `pretty_assertions` in the root crate but doesn't use it in most tests.
    With this change `pretty_assertions::assert_eq!` is used instead of
    `core::assert_eq!` for better diffs when debugging the tests.

    I thought of adding the dependency to other crates but I decided not to
    since I didn't want a huge disruptive PR :)

commit d128c0e
Author: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
Date:   Sat Apr 8 14:35:16 2023 +0200

    stdlib: use the loaded library in tests and update `README` (nushell#8811)

    Should close nushell#8809.
    this PR uses the automatically loaded library from the tests by
    replacing `use std.nu ...` with `use std ...`.

    the `README` has been updated by
    - removing the very deprencated "concrete examples"
    - fixing the `use std` and the "run the tests" sections

    the `README` can be previewed
    [here](https://github.com/amtoine/nushell/blob/refactor/stdlib/use-std-in-tests-and-update-readme/crates/nu-std/README.md)
    👍
    ```
    $nothing
    ```
    - 🟢 `toolkit test stdlib`
    ```
    $nothing
    ```

commit 60e6ea5
Author: Michael Angerman <1809991+stormasm@users.noreply.github.com>
Date:   Sat Apr 8 04:53:43 2023 -0700

    remove nu_cli crate dependency from nu_std (nushell#8807)

    now nu_std only depends on nu_parser, nu_protocol and miette
    and removes the nu_cli dependency

    this enables developers moving forward to come along and implement their
    own CLI's without having to pull in a redundant nu-cli which will not be
    needed for them.

    I did this by moving report_error into nu_protocol
    which nu_std already has a dependency on anyway....

    - `cargo fmt --all -- --check` to check standard code formatting (`cargo
    fmt --all` applies these changes)
    - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
    clippy::needless_collect` to check that you're using the standard code
    style
    - `cargo test --workspace` to check that all tests pass
    - `cargo run -- crates/nu-std/tests.nu` to run the tests for the
    standard library

    > **Note**
    > from `nushell` you can also use the `toolkit` as follows
    > ```bash
    > use toolkit.nu # or use an `env_change` hook to activate it
    automatically
    > toolkit check pr
    > ```

commit 4156077
Author: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
Date:   Sat Apr 8 13:19:09 2023 +0200

    remove the "MODULE NOT FOUND" print from the parser (nushell#8816)

    related to nushell#8765.
    should close nushell#8812.
    this PR simply removes the `prinln("MODULE NOT FOUND")` from the parser.
    no more `MODULE NOT FOUND` while typing a `use` command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request std-library Defining and improving the standard library written in nu and the core Rust ccommands tests issues to add tests or fix tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants