Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NEW] Introduce automated cross version and cross fork testing infrastructure #76

Closed
hpatro opened this issue Mar 29, 2024 · 16 comments · Fixed by #1371
Closed

[NEW] Introduce automated cross version and cross fork testing infrastructure #76

hpatro opened this issue Mar 29, 2024 · 16 comments · Fixed by #1371
Assignees
Labels
help wanted External contributions would be appreciated

Comments

@hpatro
Copy link
Collaborator

hpatro commented Mar 29, 2024

DESCRIPTION

Introduce cross version/cross fork integration testing infrastructure. With the compatibility version release and planned new major version release, it will be good to improve the testing/release certification process. This will help Valkey to prepare for release(s) more confidently and avoid pain for user(s) during migration/upgrade(s).

Example Scenario:

Issue: redis/redis#12685

Redis 7.2 introduced cluster bus message extensions feature by default and it caused failure of engine upgrade from older version (i.e. Redis 6.2 or lower) due to message broadcasted from engine running 7.2 not being compatible in older versions.

PR to fix the issue during upgrade: #52

@hpatro
Copy link
Collaborator Author

hpatro commented Mar 29, 2024

@valkey-io/core-team WDYT ?

@zuiderkwast zuiderkwast added the help wanted External contributions would be appreciated label Mar 29, 2024
@zuiderkwast
Copy link
Contributor

It would be great if anyone wants to add such tests, because we're a bit overloaded at this point. It doesn't have to be in TCL (maybe Python instead?) and it doesn't even have to be in this repo, if we're testing different forks. We can make a separate interop testing repo. It is about

  • cluster bus (nodes of different versions in the same cluster)
  • replication (primary/replica of different versions)
  • RDB and AOF files (read and write by different versions)
  • Anything else?

@pragnesh
Copy link
Contributor

pragnesh commented Apr 3, 2024

i am willing to spend time. if anyone guide me some starting point.

@madolson
Copy link
Member

madolson commented Apr 4, 2024

We can make a separate interop testing repo. It is about

I think it should be this repo, I don't think we want to start introducing tests elsewhere for now.

@zuiderkwast
Copy link
Contributor

I'm fine either way, but what id'd like to see is some prototyping to get something running. Use any language, any client lib, but be ready to discard it later.

@zuiderkwast
Copy link
Contributor

There are potential benefits of having a completely separate repo for this. It would need to check out various versions of valkey anyway (so it can't just run out of the checked out repo). It could also test valkey against various forks and versions. We can have test a cluster with mixed nodes of KeyDB, Dragonfly, Redict, Redis, Valkey, various versions. We can run redis testsuites on the valkey binary. Etc.

@suxb201
Copy link

suxb201 commented Apr 17, 2024

@madolson @zuiderkwast @pragnesh @mattsta Hi everyone! Is anyone already working on this?
We currently have a Python test script that provides the same functionality as the TCL tests, but written in Python. It has been working well for the past two years and has significantly improved the efficiency of writing and debugging tests compared to TCL.
Here is a relatively old version: https://pypi.org/project/pybbt/
If you're interested, I'm willing to spend 1-2 weeks adapting it for Valkey and writing a few test examples.

@hpatro
Copy link
Collaborator Author

hpatro commented Apr 17, 2024

@suxb201 how is the test setup/teardown done? How long does a similar tcl test takes in python?

@suxb201
Copy link

suxb201 commented Apr 18, 2024

@hpatro

  1. To create an instance, use a clear statement.
  2. Instances made inside an @subcase are safely destroyed when the @subcase ends.
  3. Instances created within an @case will be destroyed when the @case concludes.
  4. The @subcase is concurrent, which makes testing efficient.
  5. There is only one @case per file, and the tests are composed of multiple files.
  6. Logs, AOF and RDB files will be arranged in a temporary directory, making it easier to debug.

Here's a example:

from testsuite import *


# Master can replicate command longer than client-query-buffer-limit on replica
@subcase()
def replication_query_buffer_limit():
    t0 = Tair()  # create a process
    t1 = Tair()  # create another process
    t1.wait_slaveof(t0)  # call 'slaveof' and then wait for the replication connection to be set up
    t0.do("config", "set", "client-query-buffer-limit", 2000000)
    t1.do("config", "set", "client-query-buffer-limit", 1048576)  # 1024*1024 = 1mb

    value = "x" * 1100000
    ASSERT_TRUE(t0.do("set", "k", value))  # 2000000 > 1100000 > 1048576
    t0.wait_consistent()
    ASSERT_EQ(t1.do("get", "k"), value.encode())
    ASSERT_EQ(t1.digest(), t0.digest())


@case(tags=["flag0", "flag1", "flag2"])
def main():
    replication_query_buffer_limit()


if __name__ == '__main__':
    main()

@zuiderkwast
Copy link
Contributor

It seems to me that porting the tests to a different language is a different topic. I think it can be discussed here: #94.

Personally, I think it would be beneficial to create a separate interoperability-testing repo where scripts and CI jobs can

@bjosv

@madolson
Copy link
Member

Reference: https://github.com/tair-opensource/resp-compatibility. Alibaba has some compatibility testing already we should investigate.

@yangbodong22011
Copy link
Contributor

I am the owner of resp-compatibility. It was originally designed to detect the Redis version compatible with the Redis-Like system from the interface, but it is also very suitable for version compatibility testing of Redis/Valkey itself. From the discussion in the issue, it can currently do

tests YES or NO
api YES
cluster bus (nodes of different versions in the same cluster) NO(But can improve)
replication (primary/replica of different versions) NO(But can improve)
RDB and AOF files YES

We can:

  • Run on daily-ci.
  • Run this test when a new version is released.
  • Run every time a PR is submitted (about 2 minutes).

Please share any thoughts you have, thank you.

@madolson
Copy link
Member

madolson commented Aug 5, 2024

@valkey-io/core-team We chatted a bit in the core meeting and want to have a vote if we want to pull in the compatibility test listed above and run it as part of the CI.

We still need a separate effort to do interversion operability tests (clusterbus and replication).

@github-project-automation github-project-automation bot moved this from Optional for rc2 to Done in Valkey 8.0 Aug 8, 2024
@madolson
Copy link
Member

madolson commented Aug 8, 2024

@zuiderkwast Did we secretly do this?

@zuiderkwast zuiderkwast reopened this Aug 11, 2024
@zuiderkwast
Copy link
Contributor

@zuiderkwast Did we secretly do this?

I must have pocket-clicked close...

@hwware
Copy link
Member

hwware commented Aug 12, 2024

@valkey-io/core-team We chatted a bit in the core meeting and want to have a vote if we want to pull in the compatibility test listed above and run it as part of the CI.

We still need a separate effort to do interversion operability tests (clusterbus and replication).

Agree, but Daily CI is not necessary.

kronwerk pushed a commit to kronwerk/valkey that referenced this issue Jan 27, 2025
This includes a way to run two versions of the server from the TCL test
framework. It's a preparation to add more cross-version tests. The
runtest script accepts a new parameter

    ./runtest --other-server-path path/to/valkey-server

and a new tag "needs:other-server" for test cases and start_server.
Tests with this tag are automatically skipped if `--other-server-path`
is not provided.

This PR adds it in a CI job with Valkey 7.2.7 by downloading a binary
release.

Fixes valkey-io#76

---------

Signed-off-by: Viktor Söderqvist <[email protected]>
hpatro pushed a commit to hpatro/valkey that referenced this issue Feb 1, 2025
This includes a way to run two versions of the server from the TCL test
framework. It's a preparation to add more cross-version tests. The
runtest script accepts a new parameter

    ./runtest --other-server-path path/to/valkey-server

and a new tag "needs:other-server" for test cases and start_server.
Tests with this tag are automatically skipped if `--other-server-path`
is not provided.

This PR adds it in a CI job with Valkey 7.2.7 by downloading a binary
release.

Fixes valkey-io#76

---------

Signed-off-by: Viktor Söderqvist <[email protected]>
Signed-off-by: Harkrishn Patro <[email protected]>
enjoy-binbin pushed a commit to enjoy-binbin/valkey that referenced this issue Feb 2, 2025
This includes a way to run two versions of the server from the TCL test
framework. It's a preparation to add more cross-version tests. The
runtest script accepts a new parameter

    ./runtest --other-server-path path/to/valkey-server

and a new tag "needs:other-server" for test cases and start_server.
Tests with this tag are automatically skipped if `--other-server-path`
is not provided.

This PR adds it in a CI job with Valkey 7.2.7 by downloading a binary
release.

Fixes valkey-io#76

---------

Signed-off-by: Viktor Söderqvist <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted External contributions would be appreciated
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants