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

simd init ouputs to stderr #15167

Closed
yaruwangway opened this issue Feb 27, 2023 · 5 comments
Closed

simd init ouputs to stderr #15167

yaruwangway opened this issue Feb 27, 2023 · 5 comments

Comments

@yaruwangway
Copy link
Contributor

Summary of Bug

simd init outputs to stderr rather than stdout.
There might be other cmd outputs to stderr ?

Version

sdk v0.45.12, v0.45.13-ics

Steps to Reproduce

  • build simd
  • simd init > init.json init.json is empty
  • simd init > init.json 2>&1 get the init json into the file
@github-actions github-actions bot added the needs-triage Issue that needs to be triaged label Feb 27, 2023
@alexanderbez
Copy link
Contributor

Remnants of @alessio wanting everything to go to STDERR.

@yaruwangway
Copy link
Contributor Author

Hi, what's the reason ?
if this way, how about simd export ? presently, simd export goes to stdout due to the code here

@tac0turtle tac0turtle removed the needs-triage Issue that needs to be triaged label Feb 27, 2023
@alexanderbez
Copy link
Contributor

Something about the UNIX standard stating all diagnostic output should go to STDERR...I can't recall. Regardless, I dont have a preference, just as long as we're consistent :)

@yaruwangway
Copy link
Contributor Author

Hi ok, thanks! then the exception issimd init, it goes to stdout.
if this is expected, i will close this issue. Gaia will do the same 👌

@github-project-automation github-project-automation bot moved this from 📝 Todo to 👏 Done in Cosmos-SDK Feb 27, 2023
@alessio
Copy link
Contributor

alessio commented Feb 27, 2023

Remnants of @alessio wanting everything to go to STDERR.

Yup 👍

mergify bot pushed a commit that referenced this issue Mar 6, 2023
## Description

This PR introduces the `cmdtest` package, offering a lightweight wrapper around cobra commands to simplify testing CLI utilities.

I backfilled tests for the `version` command, which was an example of a very simple test setup; and for the `export` command, which was more involved due to the server and client context requirements.

I did notice that there are some existing tests for some utilities, but the `cmdtest` package follows a simple pattern that has been easy to use successfully in [the relayer](https://github.com/cosmos/relayer/blob/main/internal/relayertest/system.go) and in other projects outside the Cosmos ecosystem.

While filling in these tests, I started removing uses of `cmd.Print`, as that is the root cause of issues like #8498, #7964, #15167, and possibly others. Internal to cobra, the print family of methods write to `cmd.OutOrStderr()` -- meaning that if the authors call `cmd.SetOutput()` before executing the command, the output will be written to stdout as expected; otherwise it will go to stderr. I don't understand why that would be the default behavior, but it is probably too late to change from cobra's side.

Instead of `cmd.Print`, we prefer to `fmt.Fprint(cmd.OutOrStdout())` or `fmt.Fprint(cmd.ErrOrStderr())` as appropriate, giving an unambiguous destination for output. And the new tests collect those outputs in plain `bytes.Buffer` values so that we can assert their content appropriately.

In the longer term, I would like to deprecate and eventually remove the `testutil` package's `ApplyMockIO` method and its `BufferWriter` and `BufferReader` types, as they are unnecessary indirection when a simpler solution exists. But that can wait until `cmdtest` has propagated through the codebase more.

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] ~~provided a link to the relevant issue or specification~~
- [x] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed all author checklist items have been addressed
- [ ] confirmed that this PR does not change production code
@tac0turtle tac0turtle moved this to 🥳 Done in Cosmos-SDK Nov 16, 2023
@tac0turtle tac0turtle removed this from Cosmos-SDK Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants