As I recently mentioned on discord, I changed my mind. Do what you think is the best

work in progress. mostly done though. I need to test it a bit and maybe touch up the test cases. would like to know if you find its behaviour suitable, especially the exit protocol obviously. try changing the solution so it passes and see how it goes?

idk if X is what we're going with, I'm still waiting for us to figure what.. what the kata is supposed to be

I found a problem of sorts with b'X' as exit message:

# let's say a single recv gives:
b'abc\nXdef'
# should echo back 'abc\n' and close

which might be more than you want to ask of the solution. the exit message effectively introduces framing - a single frame followed by X (same problem as exit in other words. they both rely on framing. the send/recv calls match up 1:1 for short bytestrings locally, but without a guarantee that this is how that should behave it is still my opinion that can't be used as a poor mans framing)

the obvious solution there is to read to end of stream. but that might instead be too trivial (and fully copy-pastable from echo server examples online)

I should mention that I'm willing to fix the issues I have, it's not that I'm trying to create a bunch of work for you. I just need the framing thing figured out. no framing and b'X' for exit (or e-x-i-t found anywhere in stream, but that would be a 4 char buffer) and I'd also want to know whether the exit message should be echo'd back or not.

Or if you have other thoughts, then let's sort that out. I'm looking for something that makes sense to the both of us.

(the java translation do be bit of a problem though)

here's my suggestion for a replacement of the current exit check:

I do not guarantee that this is correct. I merely think this looks less sus.

def does_conn_get_closed(conn) -> bool:
    conn.settimeout(0.1)  # float, seconds
    try:
        # > A returned empty bytes object indicates that the client has disconnected.
        while conn.recv(1024) != b'':
            # could possibly inspect data received if wanting to test whether
            # it sends things it shouldn't
            # such as echoing back b"exit" if that is considered incorrect
            pass
    except TimeoutError:
        return False
    return True

and then for the two ports:

@test.describe('tests')  # btw its are still missing, somewhere, haven't thought about where
def basic_tests():
    threading.Thread(target=socket_server, daemon=True).start()  # don't join later, just leave it
    time.sleep(0.1)
    s = socket.socket()
    # this is a bit convoluted because I want 1111 tried first
    # but if both fail I want 1111's exception shown
    try:
        s.connect(('127.0.0.1', 1111))
    except Exception as e1111:
        s.close()  # idk if bad things can happen on failed connect, so, new fresh socket.
        s = socket.socket()
        try:
            s.connect(('127.0.0.1', 80))
        except:
            raise e1111

and then the whole framing thing still needs to be figured out. so please don't merge anything in.

my thought on that atm is that \n probably should not even be mentioned since it currently does nothing -- so, can we just treat all this like a stream that is to be echo'd back? the one incompatibility with that is that it is unclear what the format of exit should be. maybe it should be a single character, like X, or a null byte because that way there's no concern around buffers or framing. just an idea. I don't care how exit functions, so long as it makes sense. or if you can convince me that the mentioned tcp packet guarantee exists in some form.

oh and.. send is being used incorrectly - its return value is not being checked. it does not guarantee that it is going to send everything.

1: I don't necessarily think exit should end in \n, depends a bit on other choices though. this issue is about that \n is missing between all messages, it never gets sent ever.

2,3: does such a guarantee actually exist? to my (flawed) understanding the kernel will split it into packets however it pleases. a send call is not a frame, not even with short bytestrings.

4: start server exactly once, attempt to connect on both ports and see which sticks and continue to use that one without closing it

6: if it's not about framing then it probably shouldn't act on frames. so no \n at all. or if it is to act on frames, then they need to actually be used. one or the other. if the \n have meaning then there has to be a buffer. if there's no buffer, then \n have no purpose and mentioning them in the description becomes weird.

7: solver shouldn't debug test code or even have to question it. if the tests need more logic to behave well then so be it.

I'm also not at all convinced by that exit check being correct. I'd be looking for some kind of polling with a timeout here, continuing to call recv until it gives an empty response for example. I'm not happy with things that merely "seem to work"

waiting on solution threads to exit also needs timeouts. the user isn't told this is happening, so it needs to be well behaved. alternatively, don't wait, and change them to daemon threads, that way there isn't a hidden unspecified test that times out on fail.

My idea of valid issue is whether I can reproduce it from the description. I should probably say THAT instead. There are obviously things to fix.

If it's your most recently submitted code that you refer to as your code then my understanding is that you have two servers on the same port, with connections being distributed between them. So if the first one ends up getting the connection then the second one never ends up receiving exit and the tests join on that thread so that'll be what times out (aside when the tests fail to send anything at all due to 0 length words which also causes timeout) (and aside from join it would also timeout because they're non-daemon threads - preventing the process from exiting even if the tests have run to end)

I've been thinking about whether this kata is fixable, what trips me up is what to do about the incorrect framing usage because that's a significant thing that needs to change about the kata design - either remove it to keep it simple or fix the inputdata and add tests for framing. Either one would break nearly all current solutions. I need an adult.

They both serve a purpose. The text is better for direct eyeballing while the string literals are better for copying for further testing/debugging. If one of them was to be removed it's 1, because it is less exact. But there's no need for that, is there. It's not like it's in the way. What can be done though is to combine them by line breaking string literals in some way such that it can still be copied and evaluated, for example:

''.join(
 [ 'Hi, I am some \n'
 , 'text and I need \n'
 , 'to be arranged.'
 ]
)

.. the real problem here in my opinion is that the input text isn't a string literal

fixed

issue 1:

the python tests do not send '\n' to separate messages, which it promises to do. (this leads to kata timeout when 0 bytes are sent) (this also means there will never be a frame starting with b"exit" as the previous frame is not terminated with \n

issue 2:

tests may (unlikely, admittedly) generate messages starting with the bytes b"exit", for example: b"exitabcd\n" (\n would currently not be there but it's supposed to be)

correct behaviour for the server/solution is to shut down because no further bytes are expected after b"exit" as description says this is not followed by \n

issue 3:

the tests ignore framing when receiving, they don't continue to read until receiving the delimiter '\n' before doing their == comparison. it may receive only part of the frame, the server/solution may for example call send for each individual byte, which would be a correct thing to do.

issue 4:

the server is started twice. if the compatibility shim wants to try two ports it should still only start once as connecting to the wrong port won't be seen by the server anyway. this is somewhat harmless in most cases but it results in a traceback being printed when the second one fails to bind

issue 5:

tests attempt to connect immediately after starting server thread. that socket may still be closed at that time.

in the case of old solutions using port 80, the server may also fail to start due to having already been opened and closed immediately before and the socket may still be considered in use

issue 6:

I would argue that message framing should be tested. the comment about short strings should be removed as irrelevant as the framing is determined by '\n' separators.

this can be done by calling send several times in a row without \n, possibly with short delays between and maybe altering socket settings to ensure they're sent immediately so that the server is very likely to have to call recv multiple times. likewise, multiple frames can be sent in a single send call

yes, this makes the solution far more complicated than what is probably expected! but is it supposed to be correct or no?

issue 7:

unexplained timeouts from joining threads and blocking send/recv calls - it's not ok for timeouts to happen in the test code

I'm sure there are more problems, I'm not familiar with socket programming so there's lots I wouldn't spot.

I like the idea of a socket kata though ..

mine times out when the test code "sends" an empty bytestring. which isn't to spec, and I'm guessing might be a no-op so it actually didn't send anything then expects a response

this is due to word length being random.randint(0,100) (1 in 101 chance to be empty)

I guess that's also the reason for \n. except it doesn't do that.

BTW: your issue isn't exactly valid since it doesn't say what it is failing to pass or why it fails. it doesn't describe a problem with the kata. except for the part about calling the solution function twice. but that probably isnt't the issue you're having due to one harmlessly crashing and the other continuing to exist.

I'd time out sometimes.

and there's a (poor) compatibility shim which starts another server. but I'd like to think that the second server to start would harmlessly crash, I don't see how that would cause a timeout.

it seems to me like it's test.assert_equals that doesn't finish. which is.. uhm. why..

aside from that, it also doesn't terminate messages with \n as it promises. not sure why it promises that but. would be nice if that matched up.

and tests aren't inside its

can't verify due to not having solved in c++/not seeing your code
but - this looks to me like you're mixing up different test cases, ie. you're showing the input of one test and the expected output of a different one. be careful with how you print and how you read the output.

to answer the question of language, it is cpp

closing: this is straight up wrong answer with all relevant information being present in the failure message. If it was tested to say 0 locally I would chalk that up to different iteration order of unordered_map

it does however indicate weak tests. the bug they have is that they use 0 as a default value instead of eg. -1, and after setting the count to 0, they then overwrite it, mistaking it for the default

.