fyi, I was confused by how it compiled / ran as well, but I was just mirroring the existing test code.

once I update PR, tests should be good to go, as had to fix a bunch of things to get them to work now which indicates that they are being used correctly.

note that while the reply module tests are now running, I had to comment out 2 of them, to get them to work for now. I'll remove that to get this PR to fail after I see it passing

note that while the reply module tests are now running, I had to comment out 2 of them, to get them to work for now. I'll remove that to get this PR to fail after I see it passing

the map test fails because it expects doubles (with .0 suffix) and gets plain integers, i think you should just fix the test.
i.e. the test expected to get a list rather than a dict, but it didn't take into account that in resp2 these doubles are strings and not real doubles (or maybe back then the test infra handled them differently). either way, you should just fix the test.

the attribute test actually expects the error in RESP2, but it puts the catch on the wrong line (should be on the execution of the rw.attribute command, rw.error is part of another test).

please fix these.

so the map test was easy to fix, having trouble with the attribute test.

I've tried to simplify it to

        test {RM_ReplyWithAttribute: an set reply} {
            if {$proto == 2} {
                catch [r rw.attribute 3] e
                assert_match "Attributes aren't supported by RESP 2" $e
            } else {
                set res [r rw.attribute 3]
                assert_equal [dict create 0 0.0 1 1.5 2 3.0] $res
            }
        }

but it still fails on the catch in the proto == 2 run. when I comment out the proto == 2 run, it blocks forever.

i don't understand what failure you're getting. the code looks correct. maybe post the exact error?

i also don't understand why it blocks forever, IIUC you're running the resp3 branch of the test code with hello 2 so the rw.attribute is executed without a catch and it should cause the tcl code to crash (exception isn't caught)

p.s. you can use assert_error to verify the failure rather than catch it yourself.

ohh wait, i think your catch line should be using curly braces, not rectangular.
what your code currently does is execute the code in the brackets and pass the result to catch. rather than pass the script to catch so that it can execute it.

yes, already figured out the curley bracket thing. still hangs though. I was wondering if it was because of an odd number of elements in rw_attribute() (i.e. its a map in practice), the function didn't line up with the expected response as it is) so I fixed that, but still hangs.

i.e. this

int rw_attribute(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
    if (argc != 2) return RedisModule_WrongArity(ctx);

    long long integer;
    if (RedisModule_StringToLongLong(argv[1], &integer) != REDISMODULE_OK)
        return RedisModule_ReplyWithError(ctx, "Arg cannot be parsed as a integer");

    if (RedisModule_ReplyWithAttribute(ctx, integer) != REDISMODULE_OK) {
        return RedisModule_ReplyWithError(ctx, "Attributes aren't supported by RESP 2");
    }

    for (int i = 0; i < integer; ++i) {
        RedisModule_ReplyWithLongLong(ctx, i);
    }

    RedisModule_ReplyWithSimpleString(ctx, "OK");
    return REDISMODULE_OK;
}

I believe is wrong for multiple reasons, the not being a map and the OK return. However, fixing those things, doesn't cause it not to hang. but I am missing understanding of what attribute types are supposed to be. It seems to be addon data to normal requests, but that doesn't make sense as its own command/response then?

current test tcl code is

        test {RM_ReplyWithAttribute: an set reply} {
            if {$proto == 2} {
                catch {[r rw.attribute 3]} e
                assert_match "Attributes aren't supported by RESP 2" $e
            } else {
                set res [r rw.attribute 3]
                assert_equal [dict create 0 0.0 1 1.5 2 3.0] $res
            }
        }

perhaps because of the redis support code

          | {
              # ignore attributes for now (nowhere to store them)
              redis_read_map $id $fd
              continue
          }

yes, changing it to

| {return [redis_read_map $id $fd]}

means no more hanging

I have no idea why non module tests are failing. As there's no modificatoin to any code outside of module tcl tests and test module code.

ok, figured it out my change for the attribute output, needs to go all rawread

-            | {
-                # ignore attributes for now (nowhere to store them)
-                redis_read_map $id $fd
-                continue
-            }
+            | {return [redis_read_map $id $fd]}

failing a defrag test, dont know why (perhaps only happened locally, and not in github checks)