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

Add remove_all method #12

Merged
merged 9 commits into from
Nov 9, 2018
Merged

Add remove_all method #12

merged 9 commits into from
Nov 9, 2018

Conversation

Qew7
Copy link
Contributor

@Qew7 Qew7 commented Nov 2, 2018

@yegor256 see issue #6

@Qew7 Qew7 mentioned this pull request Nov 2, 2018
@@ -83,4 +83,22 @@ def test_remove_key_with_sync_false
assert(cache.exists?(:hey) == false)
assert(cache.exists?(:wey) == true)
end

def test_remove_all_with_sync
cache = Zache.new
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Qew7 we need a stronger test here. Try to use Threads and do get() and remove_all at the same time in multiple threads. See what happens.

Copy link
Contributor Author

@Qew7 Qew7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yegor256 i dont really get what are we aiming here with threads, im not really expirienced in working with them
i added multiple threads creation with creating new values in cache and removing all values from cache, tests passed succesfully, can you please explain what we were aiming to test here?

@Qew7
Copy link
Contributor Author

Qew7 commented Nov 6, 2018

@yegor256 ive remade remove_all logic, can you check it?

@yegor256
Copy link
Owner

yegor256 commented Nov 7, 2018

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Nov 7, 2018

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor
Copy link
Collaborator

rultor commented Nov 7, 2018

@rultor merge

@Qew7 @yegor256 Oops, I failed. You can see the full log here (spent 21s)

+ squash=false
+ vars=('--env=head=git@github.com:yegor256/zache.git' '--env=pull_id=12' '--env=fork=git@github.com:Qew7/zache.git' '--env=fork_branch=11' '--env=head_branch=master' '--env=pull_title=Add remove_all method' '--env=author=yegor256' '--env=scripts=( '\''export '\''\'\'''\''head=git@github.com:yegor256/zache.git'\''\'\'''\'''\'' '\'';'\'' '\''export '\''\'\'''\''pull_id=12'\''\'\'''\'''\'' '\'';'\'' '\''export '\''\'\'''\''fork=git@github.com:Qew7/zache.git'\''\'\'''\'''\'' '\'';'\'' '\''export '\''\'\'''\''fork_branch=11'\''\'\'''\'''\'' '\'';'\'' '\''export '\''\'\'''\''head_branch=master'\''\'\'''\'''\'' '\'';'\'' '\''export '\''\'\'''\''pull_title=Add remove_all method'\''\'\'''\'''\'' '\'';'\'' '\''export '\''\'\'''\''author=yegor256'\''\'\'''\'''\'' '\'';'\'' '\''export GEM_HOME=~/.ruby'\'' '\'';'\'' '\''export GEM_PATH=$GEM_HOME:$GEM_PATH'\'' '\'';'\'' )')
+ scripts=('export '\''head=git@github.com:yegor256/zache.git'\''' ';' 'export '\''pull_id=12'\''' ';' 'export '\''fork=git@github.com:Qew7/zache.git'\''' ';' 'export '\''fork_branch=11'\''' ';' 'export '\''head_branch=master'\''' ';' 'export '\''pull_title=Add remove_all method'\''' ';' 'export '\''author=yegor256'\''' ';' 'export GEM_HOME=~/.ruby' ';' 'export GEM_PATH=$GEM_HOME:$GEM_PATH' ';')
+ container=yegor256_zache_12
+ as_root=false
+ mkdir -p /home/rultor/.ssh
+ echo -e 'Host github.com\n\tStrictHostKeyChecking no\n'
+ chmod 600 /home/rultor/.ssh/config
+ git clone git@github.com:yegor256/zache.git repo
Cloning into 'repo'...
+ cd repo
+ git config user.email me@rultor.com
+ git config user.name rultor
+ '[' -z 'export '\''head=git@github.com:yegor256/zache.git'\''' ']'
+ cd ..
+ cat
+ '[' false = true ']'
+ cat
+ chmod a+x entry.sh
+ cat
+ echo 'export '\''head=git@github.com:yegor256/zache.git'\''' ';' 'export '\''pull_id=12'\''' ';' 'export '\''fork=git@github.com:Qew7/zache.git'\''' ';' 'export '\''fork_branch=11'\''' ';' 'export '\''head_branch=master'\''' ';' 'export '\''pull_title=Add remove_all method'\''' ';' 'export '\''author=yegor256'\''' ';' 'export GEM_HOME=~/.ruby' ';' 'export GEM_PATH=$GEM_HOME:$GEM_PATH' ';'
+ sensitive=()
+ rm -rf .gpg
+ cd repo
+ git remote add fork git@github.com:Qew7/zache.git
+ git remote update
Fetching origin
Fetching fork
From github.com:Qew7/zache
 * [new branch]      11         -> fork/11
 * [new branch]      3          -> fork/3
 * [new branch]      4          -> fork/4
 * [new branch]      5          -> fork/5
 * [new branch]      master     -> fork/master
 * [new branch]      ref        -> fork/ref
+ args=
+ '[' default == default ']'
+ args=' --ff'
+ '[' default == no ']'
+ '[' default == only ']'
+ export BRANCH=__rultor
+ BRANCH=__rultor
++ wc -l
++ git show-branch __rultor
+ '[' 0 -gt 0 ']'
+ git checkout -B __rultor fork/11
Switched to a new branch '__rultor'
Branch __rultor set up to track remote branch 11 from fork.
+ git checkout -B master origin/master
Switched to and reset branch 'master'
Branch master set up to track remote branch master from origin.
Your branch is up-to-date with 'origin/master'.
+ '[' false == true ']'
+ '[' false == true ']'
+ git merge --ff __rultor
Auto-merging test/test_zache.rb
Auto-merging lib/zache.rb
CONFLICT (content): Merge conflict in lib/zache.rb
Automatic merge failed; fix conflicts and then commit the result.
'cid' file is absent, container wasn't started correctly

@yegor256
Copy link
Owner

yegor256 commented Nov 7, 2018

@Qew7 there are merge conflicts, see above. Please resolve them and let me know when the branch is ready. I will try to merge again.

@Qew7
Copy link
Contributor Author

Qew7 commented Nov 7, 2018

@yegor256 ive resolved conflicts, please try to merge again

@yegor256
Copy link
Owner

yegor256 commented Nov 9, 2018

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Nov 9, 2018

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 945a8a4 into yegor256:master Nov 9, 2018
@rultor
Copy link
Collaborator

rultor commented Nov 9, 2018

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 2min)

@yegor256 yegor256 mentioned this pull request Nov 9, 2018
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.

3 participants