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

Update Redis and libraries #14

Merged
merged 4 commits into from Feb 19, 2019
Merged

Update Redis and libraries #14

merged 4 commits into from Feb 19, 2019

Conversation

kardapoltsev
Copy link
Collaborator

No description provided.

@kardapoltsev
Copy link
Collaborator Author

@Ma27 WDYT?

@Ma27
Copy link
Owner

Ma27 commented Feb 19, 2019

First of all sorry for the delay (again)! I'm currently having the following issues with this PR (but I can fix them myself):

  • I'd like to remove the ByteString assertions entirely. We already experienced behavioral differences between several platforms, so this should be dropped entirely

  • I need to find a better solution for the redis ruby script. The approach with the global gem installation never worked for me, I'm currently thinking about using i.e. bundler (or switch back to a Nix(OS) based approach).
    EDIT: just seen that thsi isn't even needed anymore which is even better, thanks!

Let's see how farI get, but I plan to merge this during the next days :)

@Ma27
Copy link
Owner

Ma27 commented Feb 19, 2019

The current problem is that there are some heavy tests that fial randomly. I experienced this before on GitLab CI and other environments. As this behavior is now observable on Travis CI as well, there's yet another reason to rework the test suite.

@Ma27
Copy link
Owner

Ma27 commented Feb 19, 2019

@kardapoltsev have you tested the test suite against Travis multiple times or just once?

As random tests with heavy IO fail, I doubt that it's related to my commits from last lights to be honest (I've experienced it before, so I doubt that this PR is related). May I ask if you have an idea how to fix it? Otherwise I'd have a more detailed look at the test suite to hopefully find a fix next week.

@kardapoltsev
Copy link
Collaborator Author

kardapoltsev commented Feb 19, 2019 via email

@Ma27
Copy link
Owner

Ma27 commented Feb 19, 2019

Hmm I see. If it's okay for you I'd merge this for now as getting rid of redis-trib.rb is IMHO a great step forward in our test suite. To get those tests fixed I guess that we need to improve our tests that do heavy IO.

@kardapoltsev
Copy link
Collaborator Author

kardapoltsev commented Feb 19, 2019 via email

@Ma27 Ma27 merged commit 0c7faba into Ma27:master Feb 19, 2019
@Ma27
Copy link
Owner

Ma27 commented Feb 19, 2019

@kardapoltsev thanks a lot! Unless anybody else is faster, I'll have a deeper look at the tests next month :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants