[sui CLI] Unit test Sui cli

[patrickkuo]

• Make cli testable by replacing println with writer
• Using PathBuf instead of String in config
• added test for Sui genesis
• Some refactoring

[huitseeker]

YAY LOGS! 🤩

I left one indication on the use of tracing which I think may help.

more details:

• the renames are nice, ruplacer is useful if you want to do more codemods to tie up loose ends,
• the PathBuf use is nice, camino is nice if you have to do path surgery,

[huitseeker]

So this is certainly an improvement on the existent logging, thanks! But it's also choosing the absolute simplest form of logging, which wouldn't be so much of a problem if it wasn't a bit hard to evolve:

• the printouts you send have no level attached to them, so that you get all the prints or none at all,
• you log to a single blocking writer, with no flexibility to route your logs to several places (files, network, stdout/stderr),

If one day we want to change either one of those things, I'm afraid it may be a large refactor.

If you set it this way instead:

tracing_subscriber::fmt::init();

Which is essentially shorthand for:

use std::io;

let subscriber = tracing_subscriber::fmt::Subscriber::builder()
    .with_writer(io::stdout)
    .finish();
 tracing::subscriber::set_global_default(subscriber)
    .expect("setting tracing default failed");   

Then you're not limited to writeln!, you can do:

let span = span!(Level::INFO, "Starting network with {} authorities", config.authorities.len());
let _enter = span.enter();

and later

tracing::info!("All servers stopped");

It doesn't sound like much but:

• log-level tweaking is easy, and the calling context is preserved.
• It is thankfully straightforward to override the default subscriber for the course of one function and its descendants.
• Moreover, much later in the course of development of the CLI, we can e.g. hook up appender as the subscriber and get non-blocking rolling logs for the CLI, for free: https://crates.io/crates/tracing-appender

[patrickkuo]

this is much nicer then passing in a writer! I will use tracing instead!

[huitseeker]

This is awesome! 🚀

[huitseeker]

You're right, I messed up by not noticing the to_str() is a borrow-through. Sigh. This is why I prefer camino. Here's what I would write (this is a total nit by now).

Suggested change

	let files = read_dir(working_dir.path())?
	.flat_map(|r| r.map(|file| file.file_name().to_str().unwrap().to_owned()))
	.collect::<Vec<_>>();
	assert_eq!(3, files.len());
	assert!(files.contains(&"wallet.conf".to_string()));
	assert!(files.contains(&"authorities_db".to_string()));
	assert!(files.contains(&"network.conf".to_string()));
	let files = read_dir(working_dir.path())?
	.flatten()
	.flat_map(|file| file.file_name().into_string())
	.collect::<Vec<_>>();
	assert_eq!(3, files.len());
	assert!(files.contains(&"wallet.conf".to_owned()));
	assert!(files.contains(&"authorities_db".to_owned()));
	assert!(files.contains(&"network.conf".to_owned()));

[huitseeker]

OK, still a minimalist approach. Why not?

[huitseeker]

Aha! I see you've found some of the nicer features already :D