thomasarmel / socket-server-mocker

Mock socket server in Rust, for testing various network clients.
https://crates.io/crates/socket-server-mocker
MIT License
0 stars 2 forks source link

A few small lints #7

Closed nyurik closed 1 week ago

nyurik commented 1 week ago

Note that you can use #![doc = include_str!("../README.md")] trick in the lib.rs to include README as the crate documentation. This way it the tests in README will be validated, and the content will only be used once.

thomasarmel commented 1 week ago

Nice thank you very much! It looks like it solves issue #6 then...

I'll publish the new version so

nyurik commented 1 week ago

Thanks! What do you think about naming? Would you be OK if I made some name changes to make code more readable?

thomasarmel commented 1 week ago

Depends on what you want to change. Could you give some examples? Personally speaking, I prefer precise over concise naming, so for example, and it's my personal opinion, I think renaming ServerMockerInstructionsList to something like SMInstrList is not a good idea, as it makes the code more confusive. So please tell me what you want to change.

Btw at the time I started this project I was a Rust newbie so I understand there could be some bad practices, I would be grateful for any advice 😄

thomasarmel commented 1 week ago

Btw #6 isn't solved but it seems to occur less often 🤔

nyurik commented 1 week ago

Heh, not certain of the #6 - I haven't looked at why it might be failing, so I was surprised you said it fixed things.

Naming is hard, I agree. I think HTTP mock server mockito has a fairly well defined, concise API that you might want to mimic.

For example, this I believe is pretty verbose, to the point of actually being unreadable:

mock.add_mock_instructions_list(ServerMockerInstructionsList::new_with_instructions(&[
  ServerMockerInstruction::SendMessageDependingOnLastReceivedMessage(...)
]));

I would get rid of the ServerMockerInstructionsList all together -- you don't need to pass a struct to the mock, you can just give it the commands. At minimum, it would be something like this (work in progress, subject to discussion and ideas):

// Create fake UDP server
let server = FauxUdpServer::with_port(4321).unwrap(); // specific port
let server = FauxUdpServer::with_port_v6(4321).unwrap(); // specific port on IPv6
let server = FauxUdpServer::new().unwrap(); // any port
let server = FauxUdpServer::new_v6().unwrap(); // any port on IPv6

// get server port -- there is only one type of port for a server, keep things simple
let port = server.port();

// the string representation of the server address
let port = server.host_with_port();

// Sometimes users may want the whole socket - easier to just return that
// returns SocketAddr that includes the port. Note that it contains both V4 and V6 variants
let socket = server.socket_address();

// Add mock instructions
server.receive_message();
server.send_message(&[1, 2, 3]);
server.receive_message_smaller_than(1024);
server.custom_responder(|msg| {
    if msg == b"foo" {
        Some(b"bar".to_vec())
    } else {
        None
    }
});
server.stop();

Better yet, if you adapt the mockito API, it could even be something like I described in https://github.com/lipanski/mockito/issues/204 -- perhaps we could merge the two projects?

thomasarmel commented 1 week ago

Ok looks an interesting idea! I have plenty of things to do these days so it won't be my top priority but every contribution is welcomed 😄 .

Do you use this crate for specific needs?