Add Redis module #642
Add Redis module #642
Conversation
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Hi @cristianrgreco, it's ready for review. |
|
Hi @cristianrgreco, who can I ask for a review? There's no contribution guide lines or a how to. |
|
Hi @djakielski, I've been away the last couple of weeks. I'm back now and will have a look. Thanks for the contributions! |
cristianrgreco
added
enhancement
|
|
||
| export class RedisContainer extends GenericContainer { | ||
| private password = ""; | ||
| private persistenceVolume = ""; |
There was a problem hiding this comment.
Let's use an optional type private persistenceVolume?: string and then check if (this.persistenceVolume) to get type-checking support. We could do the same with the password
There was a problem hiding this comment.
Only nitpicks - a great contribution, thank you!
I'll need to think of something else to use as the Quickstart example 😄
Co-authored-by: Cristian Greco <cristianrgreco@gmail.com>
Co-authored-by: Cristian Greco <cristianrgreco@gmail.com>
|
Hi @cristianrgreco, how can i start all these test again? I had fixed an issue and want to retest it. |
|
Nice contribution! |
There was a problem hiding this comment.
Thanks for your contribution, @djakielski ! I've left some comments.
| ...(this.persistenceVolume ? ["--save 1 1 ", "--appendonly yes"] : []), | ||
| ]) | ||
| .withStartupTimeout(120_000); | ||
| if (this.persistenceVolume) this.withBindMounts([{ mode: "rw", source: this.persistenceVolume, target: "/data" }]); |
There was a problem hiding this comment.
I don't think this is something we would like to do. Testcontainers encourages to use copyFilesToContainer instead and make sure the tests are portable across docker environments.
There was a problem hiding this comment.
I was thinking about persisting Tests. But on the view of making tests independent, you're right. I will fix it.
There was a problem hiding this comment.
@djakielski AFAIK this is the only change pending
There was a problem hiding this comment.
@cristianrgreco I had some trouble with correct way of importing data via redis cli when executing command in the container. I solved it via bash script.
|
@cristianrgreco do you have any idea why the tests are failing? What did i wrong? |
|
@djakielski I've noticed some intermittent issues with the GHA runners over the last few days. I've kicked off the tests again, hopefully the issue is now resolved. Nothing related with this PR |
|
@cristianrgreco could you rerun the tests? Seems like another temporary issue. |
|
Thanks for your patience and effort @djakielski, much appreciated! |
Adds new testcontainer for redis
Task: