standard library: add log commands by presidento · Pull Request #8448 · nushell/nushell

presidento · 2023-03-14T16:27:03Z

Description

log critical "this is a critical message"
log error "this is an error message"
log warning "this is a warning message"
log info "this is an info message"
log debug "this is a debug message"

Tests + Formatting

Tests are written. To run automatically, #8443 needs to be merged before or after this PR.

amtoine · 2023-03-14T17:49:09Z

this looks cool 😮 👍

however i'm not sure how to run this 🤔

like could you explain a bit more how to run your example above? 😋
i keep getting error or integer multiples of 10 😕

i check the source code as soon as i can run you log commands 😌

presidento · 2023-03-14T20:31:44Z

Sorry, I just left the * from std.nu import.
Now it should work. If not, please attach the error message.

❯ cd crates\nu-utils\standard_library\

❯ use std.nu *

❯ log info test
INFO  test

❯ use test_logger.nu example_log

❯ example_log
CRIT  this is a critical message
ERROR this is an error message
WARN  this is a warning message
INFO  this is an info message

❯ NU_LOG_LEVEL=DEBUG example_log
CRIT  this is a critical message
ERROR this is an error message
WARN  this is a warning message
INFO  this is an info message
DEBUG this is a debug message

amtoine · 2023-03-15T17:20:59Z

Sorry, I just left the * from std.nu import.

no worries, std log ... is perfect 👌

Now it should work. If not, please attach the error message.

i does not want to work it appears 😮

you can see this recording to see what happens, following your instructions 😌

i might be missing something dumb, but i think i follow exactly what you gave me!
tell me if there is anything i can try on my side to fix this 😉

It is broken (no longer supported?) with Nushell 0.77

codecov · 2023-03-16T06:59:42Z

Codecov Report

Merging #8448 (ed27f00) into main (8543b07) will decrease coverage by 0.09%.
The diff coverage is n/a.

❗ Current head ed27f00 differs from pull request most recent head 644ba05. Consider uploading reports for the commit 644ba05 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8448      +/-   ##
==========================================
- Coverage   68.52%   68.43%   -0.09%     
==========================================
  Files         621      621              
  Lines      100124    99958     -166     
==========================================
- Hits        68608    68408     -200     
- Misses      31516    31550      +34

see 12 files with indirect coverage changes

presidento · 2023-03-16T07:33:41Z

Thank you very much for the recording. It helped me a lot. Now it should work. (It seems that the return command has an unusual behaviour in 0.77: #8472)

presidento · 2023-03-16T09:10:58Z

Please note that it also can be used within pipelines

def my-command [] {
    let input = $in
    log info $"This is a log message. Input is: ($input)"
    $input
}

echo "Hello My World" | my-command | split row " "

fdncred · 2023-03-16T12:16:58Z

Are we ready to land this one?

amtoine · 2023-03-16T17:30:09Z

Thank you very much for the recording. It helped me a lot. Now it should work. (It seems that the return command has an unusual behaviour in 0.77: #8472)

coool that works now 🥳

Please note that it also can be used within pipelines

and that is really nice to be able to do that 👌

amtoine · 2023-03-16T17:30:31Z

i'll just have a quick look at the source code, but i think we're very close 💪

amtoine

only minor things for the most part.

however i have a problem 🤔
i've had a look at your tests and they look to cover the things we expect from the logger 👍
the thing is they do not run properly 😮

use crates/nu-utils/standard_library/test_logger.nu

test_logger test_critical
test_logger test_error
test_logger test_warn
test_logger test_info
test_logger test_debug

gives me

CRIT  test message
Error: nu::shell::only_supports_this_input_type

  × Input type not supported.
    ╭─[crates/nu-utils/standard_library/test_logger.nu:8:1]
  8 │ def "assert message" [level_str: string, output:string] {
  9 │     assert ($output | str contains $level_str)
    ·             ───┬───   ──────┬─────
    ·                │            ╰── only string input data is supported
    ·                ╰── input type: nothing
 10 │     assert ($output | str contains "test message")
    ╰────

i think this might need to be fixed before merging, right? 😋

crates/nu-utils/standard_library/logger.nu

crates/nu-utils/standard_library/test_logger.nu

presidento · 2023-03-16T19:08:35Z

If it's OK, I would like to rebase and use log command in test runner. Or is it better to send another PR for this small change?

presidento · 2023-03-16T19:17:29Z

Do you want me to put log commands to std.nu as well? (See #8489)

fdncred · 2023-03-16T23:06:47Z

@presidento - In order to distribute these files in the next release (hopefully) we'll need all of our file inside of std.nu all working harmoniously together.

presidento · 2023-03-17T08:21:18Z

@fdncred 🙌 Thank you for having auto-distributed standard library so quickly.

I merged main to this branch and moved log commands to std.nu.

amtoine

thanks for the doc, for the fixes and for putting this in std.nu, saves a bit of work 😌

i still have an issue with the tests however...

>_ rm crates/nu-utils/standard_library/test_std.nu  # temporary command, see warning below
>_ rm crates/nu-utils/standard_library/test_dirs.nu  # temporary command, see warning below

>_ nu crates/nu-utils/standard_library/tests.nu
INFO  Run tests in test_logger module
Error:
  × Assertion failed.
   ╭─[/home/amtoine/.local/share/git/store/github.com/amtoine/nushell/crates/nu-utils/standard_library/test_logger.nu:7:1]
 7 │     let output = (run $system_level $message_level)
 8 │     assert ($output == "")
   ·            ───────┬───────
   ·                   ╰── It is not true.
 9 │ }
   ╰────

Warning
the test_dirs and test_std do not pass on the latest main, so i had to remove these test modules
HOWEVER, this does NOT have anything to do with this PR, i think it can be saved for later 😉

presidento · 2023-03-17T17:26:34Z

It works for me for nu v0.77 and 0.76. I assume #8504 will solve this, as well. I changed assertion to have better message.

amtoine · 2023-03-17T17:37:06Z

It works for me for nu v0.77 and 0.76. I assume #8504 will solve this, as well. I changed assertion to have better message.

it does not solve it...

however, i trust you!
it might just be a problem with latest main 🤔

not sure what to do 🤔

land this and fix later
fix this for 0.77.2 right now

@fdncred
what should we do here, 'cause i can not run the tests to completion? 😋

fdncred · 2023-03-17T17:41:09Z

ummm, tell me how to test it and i'll see what happens for me

amtoine · 2023-03-17T17:57:43Z

ummm, tell me how to test it and i'll see what happens for me

checkout on the PR branch and run

nu crates/nu-utils/standard_library/tests.nu

you should have no error

fdncred · 2023-03-17T19:16:39Z

@amtoine This is what I get

nu crates\nu-utils\standard_library\tests.nu 
INFO  Run tests in test_dirs module
INFO  Run tests in test_logger module
Error:
  × Assertion failed.
   ╭─[C:\Users\username\source\repos\forks\nushell\crates\nu-utils\standard_library\test_logger.nu:7:1]
 7 │     let output = (run $system_level $message_level)
 8 │     assert eq $output ""
   ·               ─────┬────
   ·                    ╰── They are not equal: Error: nu::parser::module_not_found

  × Module not found.
   ╭─[source:1:1]
 1 │ use std.nu; NU_LOG_LEVEL=99 std log critical "test message"
   ·     ───┬──
   ·        ╰── module not found
   ╰────
  help: module files and their paths must be available before your script is
        run as parsing occurs before anything is evaluated

 !=
 9 │ }
   ╰────


INFO  Run tests in test_std module

presidento · 2023-03-17T19:25:55Z

Oh, sorry. I'll fix it.

Chdir to the folder of std.nu, since it must be run as an external command, so the usual import logic does not work.

presidento · 2023-03-17T20:14:58Z

I'm sorry for the additional work. Asserting for the output of print command is tricky. It worked before, when I used echo, later I always tested it staying in the directory of standard library.

presidento · 2023-03-17T20:15:28Z

I resolved the conflicts in std.nu (dirs is merged into it).

fdncred · 2023-03-17T20:55:11Z

I'm not sure what's going on but I can't get it to work

nu crates\nu-utils\standard_library\tests.nu
Error: nu::shell::unsupported_input

  × Unsupported input
   ╭─[source:1:1]
 1 │ nu -c "use C:\\Users\\username\\source\\repos\\forks\\nushell\\crates\\nu-utils\\standard_library\\test_dirs.nu *; $nu.scope.commands | to nuon"
   · ▲
   · ╰── input type: value originates from here
   ╰────
   ╭─[source:1:1]
 1 │ use C:\Users\username\source\repos\forks\nushell\crates\nu-utils\standard_library\test_dirs.nu *; $nu.scope.commands | to nuon
   ·                                                                                                                        ───┬───
   ·                                                                                                                           ╰── custom values are currently not nuon-compatible
   ╰────

Error: nu::shell::only_supports_this_input_type

  × Input type not supported.
    ╭─[C:\Users\username\source\repos\forks\nushell\crates\nu-utils\standard_library\tests.nu:7:1]
  7 │             nu -c $'use ($test_file) *; $nu.scope.commands | to nuon'
  8 │             | from nuon
    ·               ────┬────
    ·                   ╰── input type: nothing
  9 │             | where module_name == $module_name
    ·               ──┬──
    ·                 ╰── only list, binary, raw data or range input data is supported
 10 │             | where ($it.name | str starts-with "test_")
    ╰────

This is the version I'm using (latest main branch)

key	value
version	0.77.2
branch	main
commit_hash	`7095d89`
build_os	windows-x86_64
build_target	x86_64-pc-windows-msvc
rust_version	rustc 1.66.1 (90743e729 2023-01-10)
rust_channel	1.66.1-x86_64-pc-windows-msvc
cargo_version	cargo 1.66.1 (ad779e08b 2023-01-10)
build_time	2023-03-17 07:06:22 -05:00
build_rust_channel	release
features	default, zip
installed_plugins	custom-value generate, custom-value generate2, custom-value update, from eml, from ics, from ini, from parquet, from plist, from vcf, gstat, hist, inc, json path, nu-example-1, nu-example-2, nu-example-3, periodic-table, plot, pnet, query, query json, query web, query xml, regex, xyplot

fdncred · 2023-03-17T20:57:19Z

oh, i bet it's because i have nushell installed with cargo install --path . --features=dataframe and dataframes use custom values.

presidento · 2023-03-17T21:07:35Z

This PR has nothing about test_dirs which failed. I assume that it does not work for you on main branch either.

fdncred · 2023-03-17T21:15:32Z

@presidento I'm just running this `nu crates/nu-utils/standard_library/tests.nu

it fails the same as above. the tests.nu script is iterating through all tests. If there's some specific command you want me to run, please let me know what it is.

presidento · 2023-03-17T22:16:46Z

It is weird... It works for me:

Does $nu.scope.commands | to nuon work for you?

I submitted a change to the tests.nu (unrelated to this PR), I hope with that it works for you as well.

amtoine · 2023-03-18T08:39:38Z

@presidento

on the ef7fbf4bf9287597175b98082e5273765f828c91 revision of nushell, i.e. almost latest main, i also get

>_ nu crates/nu-utils/standard_library/tests.nu
INFO  Run tests in test_dirs module
INFO  Run tests in test_logger module
INFO  Run tests in test_std module

if you tell me that's expected, i'll approve this PR 👌

presidento · 2023-03-18T09:31:04Z

Yes, that is expected. You can run it with NU_LOG_LEVEL=DEBUG nu crates\nu-utils\standard_library\tests.nu to see the debug logs, I updated the README, too.

amtoine

Yes, that is expected. You can run it with NU_LOG_LEVEL=DEBUG nu crates\nu-utils\standard_library\tests.nu to see the debug logs, I updated the README, too.

oooooh yes, that is really nice 😮

fantastic, thanks and let's land this i think 🥳

fdncred · 2023-03-18T13:19:50Z

Looks good, works for me. Thanks!

presidento added 3 commits March 14, 2023 17:21

standard library: add log commands

4372525

Include logger commands to std module

e8acf64

Accept short log names as levels for $env.NU_LOG_LEVEL

773dd32

fdncred added the A:std-library Defining and improving the standard library written in Nu label Mar 14, 2023

Fix missing import to std.nu

32b2689

presidento added 3 commits March 16, 2023 07:27

Do not use return <value> statement

d54f2a6

It is broken (no longer supported?) with Nushell 0.77

Change example order

dfdbb9e

Use default_dimmed for debug lines

608c3d5

presidento mentioned this pull request Mar 16, 2023

stdlib: add test discovery, extract test files #8443

Merged

Print to stderr instead of echoing to stdout

0a30391

amtoine requested changes Mar 16, 2023

View reviewed changes

Review fixes

ed27f00

presidento added 3 commits March 17, 2023 09:10

Merge remote-tracking branch 'official/main' into std-logger

482c430

Merge logger.nu submodule to std.nu

d053cc4

Use log commands in test runner

d307f25

amtoine requested changes Mar 17, 2023

View reviewed changes

Use assert eq for better messages

121c234

presidento added 2 commits March 17, 2023 21:10

Fix logger test

e68984c

Chdir to the folder of std.nu, since it must be run as an external command, so the usual import logic does not work.

Merge remote-tracking branch 'official/main' into std-logger

90b8928

tests: Get only necessary columns from the inner process

644ba05

amtoine approved these changes Mar 18, 2023

View reviewed changes

amtoine added a commit to amtoine/nushell that referenced this pull request Mar 18, 2023

do not fix the echos => being done in nushell#8448 already

78d3e29

presidento mentioned this pull request Mar 18, 2023

standard library: use the standard assert and fix test output #8509

Merged

fdncred merged commit 10a42de into nushell:main Mar 18, 2023

presidento deleted the std-logger branch April 6, 2023 09:43

coderabbitai bot mentioned this pull request Mar 23, 2026

ci: Convert GHA workflows to Woodpecker CI pipelines DaRacci/nix-config#546

Open

Conversation

presidento commented Mar 14, 2023

Description

Tests + Formatting

Uh oh!

amtoine commented Mar 14, 2023

Uh oh!

presidento commented Mar 14, 2023

Uh oh!

amtoine commented Mar 15, 2023

Uh oh!

codecov bot commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

presidento commented Mar 16, 2023

Uh oh!

presidento commented Mar 16, 2023

Uh oh!

fdncred commented Mar 16, 2023

Uh oh!

amtoine commented Mar 16, 2023

Uh oh!

amtoine commented Mar 16, 2023

Uh oh!

amtoine left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

presidento commented Mar 16, 2023

Uh oh!

presidento commented Mar 16, 2023

Uh oh!

fdncred commented Mar 16, 2023

Uh oh!

presidento commented Mar 17, 2023

Uh oh!

amtoine left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

presidento commented Mar 17, 2023

Uh oh!

amtoine commented Mar 17, 2023

Uh oh!

fdncred commented Mar 17, 2023

Uh oh!

amtoine commented Mar 17, 2023

Uh oh!

fdncred commented Mar 17, 2023

Uh oh!

presidento commented Mar 17, 2023 via email

Uh oh!

presidento commented Mar 17, 2023

Uh oh!

presidento commented Mar 17, 2023

Uh oh!

fdncred commented Mar 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fdncred commented Mar 17, 2023

Uh oh!

presidento commented Mar 17, 2023

Uh oh!

fdncred commented Mar 17, 2023

Uh oh!

presidento commented Mar 17, 2023

Uh oh!

amtoine commented Mar 18, 2023

Uh oh!

presidento commented Mar 18, 2023

Uh oh!

amtoine left a comment

Choose a reason for hiding this comment

Uh oh!

fdncred commented Mar 18, 2023

Uh oh!

codecov bot commented Mar 16, 2023 •

edited

Loading

amtoine left a comment •

edited

Loading

fdncred commented Mar 17, 2023 •

edited

Loading