valkey-io / valkey

A new project to resume development on the formerly open-source Redis project. We're calling it Valkey, since it's a twist on the key-value datastore.
https://valkey.io
Other
14.51k stars 516 forks source link

[NEW] Don't require tcl for tests #94

Open yonas opened 3 months ago

yonas commented 3 months ago

make test results in You need tcl 8.5 or newer in order to run the Redis test.

It would be great if the tests were compiled as go program(s) instead of tcl scripts.

mattsta commented 3 months ago

compiled as go program(s)

what? that's probably even worse.

instead of tcl scripts.

over time that would be nice, but the more I look at the state of the test environment, the less it looks easy to manage, so we're stuck in a cycle of extending outdated things forever.

I think one secret with the "tests" is they aren't just "tests" but rather an entire ecosystem of automation frameworks hand-written bespoke and customized for a single-purpose use case here nothing else in the world uses. The tcl test suite implements a full custom redis wire protocol client, async testing, it edits config files and deletes config files, manages test server process startup/shutdown lifecycles, and a hundred other things:

$ tokei tests
===============================================================================
 Language            Files        Lines         Code     Comments       Blanks
===============================================================================
 C                      40        11125         8376          914         1835
 Makefile                1           84           70            2           12
 Markdown                1           63            0           48           15
 TCL                   205        58263        47003         4119         7141
 Plain Text              1          111            0           94           17
===============================================================================
 Total                 248        69646        55449         5177         9020
===============================================================================

the eventual replacement process would probably be: build tests for new features under a new testing framework while maintaining all the old testing infrastructure, then over time build more new tests to replace old tests until all old tests are fully duplicated in a new system.

but, again, these aren't just "tests" but also an entire automation framework as well, so it's not as easy as just converting tests into pytest. we'd have to make an entire new automated testing architecture (though, we could probably organize it better conceptually so it better splits out use cases of service testing vs. command testing vs. interface testing vs. network protocol testing instead of having them all mixed together).

I imagine a full time attempt to port everything to a new maintainable long-term modern/reusable/standard testing framework would take around 6 months of full time work for one or two people (step 1: read all 70,000 lines of current tests; step 2: plan new logical test automation framework abstractions; step 3: implement new test automation frameworks in a 'build->use->refactor' cycle to refine incorrect assumptions; step 4: start porting tests and continue refactoring frameworks as more edge cases are found; ...).

yonas commented 3 months ago

compiled as go program(s) what? that's probably even worse.

Why would it be worse to use Go instead of tcl?

My vote goes towards Go because it will already be installed.

princ45 commented 3 months ago

every idea is good and every good result came from a good idea

N-R-K commented 3 months ago

My vote goes towards Go because it will already be installed.

$ go
zsh: command not found: go

Have you considered that not every environment is the same as whatever you're using?

yonas commented 2 months ago

Oh, I thought Redis was written in Go.

I'm not aware of the best testing framework written in C, but whatever it is, I'd recommend using it instead of tcl.

PingXie commented 2 months ago

I think one secret with the "tests" is they aren't just "tests" but rather an entire ecosystem of automation frameworks hand-written bespoke and customized for a single-purpose use case here nothing else in the world uses. a full time attempt to port everything to a new maintainable long-term modern/reusable/standard testing framework

Agreed. Switching to a new (integration) test framework is a major decision for the team. I don't think we will ever have the resource to rewrite all the tests from scratch. I have also seen many times where the new system, which is supported to replace the old system, failed to provide feature parity, and the team ended up supporting both. Once we start adding a new test framework, it is going to get worse before it can get better, if ever. Also as much I like to see tcl being phased out, this is IMO a relatively low-pri item in the next 6-12 months.

Btw, I think we actually need a true unit test framework soon, something like https://github.com/google/googletest, which complements the integration test framework, as opposed to replacing it.

madolson commented 2 months ago

Also as much I like to see tcl being phased out, this is IMO a relatively low-pri item in the next 6-12 months.

I think it was said elsewhere, but I agree with the strategy of building the new infrastructure out and then adding features to it as we need and just writing the new tests. Otherwise we'll keep adding more stuff to the old framework.

Btw, I think we actually need a true unit test framework soon, something like https://github.com/google/googletest, which complements the integration test framework, as opposed to replacing it.

I just don't want to also add C++ to the language folks need to understand.

PingXie commented 2 months ago

Otherwise we'll keep adding more stuff to the old framework.

Any modern language work for me but I am not an expert in this area. My concern is stuck with supporting two test frameworks. I think the first step is to investigate the best replacement and make sure it actually has a chance to completely replace tcl before pulling the trigger.

I just don't want to also add C++ to the language folks need to understand.

@madolson I remember you tried adding unit tests before. Which framework is that? I agree with not adding C++ dependency. gtest is just my vicinity bias.

daniel-house commented 2 months ago

I agree that we need true unit tests. However, the code is so highly optimized that it is difficult to write isolated tests for many of the functions.

I think the existing TCL system is both beautiful and ugly. The TCL framework takes advantage of some of the most dangerous features of any eval-loop language I've ever seen to provide some truly amazing features (e.g., starting 20 servers in parallel to provide test isolation and execute "make test" in just 10m13s on my ubuntu development vm). Some of the support code makes my eyes hurt, yet those are the parts that make so many of the tests so elegant. You could say I am in a love-hate relationship with the TCL end-to-end integration testing. It gives wonderful output from the tests, but doesn't bother to put any of that enormous output into a file. I could go on and on about it forever.

Please do not underestimate the cost of eliminating TCL.

mattsta commented 2 months ago

Yes, exactly all those parts.

the code is so highly optimized that it is difficult to write isolated tests for many of the functions.

I seem to recall many functions and features don't have unit tests because some of the data structures directly write their results into the client buffer. There is a lot of untestable code because it wasn't designed to be run outside of a live server in the first place (so the layering violation saves some function calls, but it makes the system untestable outside of integration testing).

I think the existing TCL system is both beautiful and ugly.

Absolutely that too. It works well for a single-purpose system if you have creators and maintainers dedicating their life to this single system. The main issue then becomes: who wants to burn their personal life experience points learning and maintaining a single-purpose system with no transferable skills to anything else in the world?

From a practical point of view, trying to find a job with heavy C experience on your resume is already near impossible (and soon illegal apparently if the rust evangelists continue to get their way!), then adding "I maintained a single-purpose tcl testing codebase" to a resume doesn't help advance marketability either.

I just try to mix my concerns for a project's long-term sustainability (this project is already 15 years old, so what does it look like in another 20 years? 40 years? when was the last time you went to the big tcl meetup in your city? oh, right.) with also looking out for the people working on the project so people don't go "dev tunnel vision" too much and forget to maintain hire-able skills (and don't end up like me, with 20+ years of experience in specialized hardware, networking, database, and observability systems nobody with hiring authority seems to care about anymore).

zuiderkwast commented 2 months ago

Personally, I think the TCL tests serve their purpose well. Spending a lot of resources translating what we have into a different language just because not everyone likes TCL is not a very good way to use our resources IMO. I need better arguments to be convinced.

Test cases are very simple programs and you don't need to understand the weirdest parts of TCL to add tests for new features. When I started contributing to Redis, I learned what I needed quite fast by looking at existing test cases.

It is very common for C projects to have tests written in a different language. OpenSSL tests are written in Perl. Others use Python. Writing tests in C is very cumbersome.

If there are good reasons to write something new, such as interoperability testing, I'm all for doing it in Python. It's familiar to many and installed in most systems.

zuiderkwast commented 2 months ago

Just in case not everyone knows why the tests are written in TCL.

Salvatore "antirez" was big fan of TCL. He blogged Tcl the Misunderstood and he wrote picol, a Tcl interpreter in 550 lines of C code and the more serious Jim TCL interpreter. Many of the Valkey internals are originally from Jim (e.g. adlist).

The predecessor of Redis was written in TCL and it was called LLOOGG Memory DB. Here's the source code in 319 lines.

zuiderkwast commented 2 months ago

... and the book "An introduction to the Tcl programming language": http://invece.org/tclwise/

daniel-house commented 2 months ago

The online reference is easily found and was all I needed. https://www.tcl-lang.org/

Don't forget, no matter what language you use, 10 years from now everyone will think you were an idiot for not using their favorite language.

Of course, 1 week from now 80% of the internet will think you are an idiot no mater what you do.