-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
in_tail: Untracked files should be removed from watching list. fix #1455 #1467
Conversation
@t-osakada Could you try this patch on your environment? |
@tagomoris Could you check this patch? |
@repeatedly refer the issue which this patch will solve about |
Ah, fix #1455 in the title doesn't refer it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add tests.
@repeatedly I confirmed that it solved. Thanks! |
@tagomoris Do you mean need test to count Ruby's object, e.g. TimerWatcher? |
@repeatedly right. |
test added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments on test code.
Please add fixes for it, and go forward.
test/plugin/test_in_tail.rb
Outdated
|
||
assert_equal 1, d.instance.instance_variable_get(:@tails).keys.size | ||
File.unlink("#{TMP_DIR}/tail.txt") | ||
sleep 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be waiting(5){ sleep 0.1 until the_size_of_tails == 0 }
instead of just sleep 2
for stability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is good. Will fix.
test/plugin/test_in_tail.rb
Outdated
sleep 2 | ||
assert_equal 0, d.instance.instance_variable_get(:@tails).keys.size | ||
|
||
base_num = count_timer_object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to confirm that the number of timer watcher does not increase anymore, right?
Please add the comment what you are checking if so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why comment is needed? Hard to understand from the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The name of test case doesn't explain what to do here.
6aa343c
to
4680963
Compare
be60a57
to
030045a
Compare
Fix unstable test on windows. Let's merge. |
No description provided.