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

[Feature #16513] TracePoint#inspect returns "... file:line" #3391

Merged

Conversation

@nguyenquangminh0711
Copy link
Contributor

@nguyenquangminh0711 nguyenquangminh0711 commented Aug 5, 2020

This PR is to resolve ticket https://bugs.ruby-lang.org/issues/16513, so that TracePoint#inspect returns path:line without @ prefix. This change ensures the consistency with previous changes in https://bugs.ruby-lang.org/issues/16412 and https://bugs.ruby-lang.org/issues/16412.

Before

#<TracePoint:line@test1.rb:12>
#<TracePoint:call `random_method'@test1.rb:12>

After

#<TracePoint:line test1.rb:12>
#<TracePoint:call `random_method' test1.rb:12>
@ko1 ko1 requested a review from eregon Aug 5, 2020
@ko1
Copy link
Contributor

@ko1 ko1 commented Aug 5, 2020

Seems nice.
@eregon could you check rubyspec parts?

@marcandre
Copy link
Member

@marcandre marcandre commented Aug 5, 2020

@eregon could you check rubyspec parts?

I'm not him, but rubyspec parts LGTM 👍

thread.join
end

inspect.should == "#<TracePoint:thread_end #{thread_inspection}>"

This comment has been minimized.

@eregon

eregon Aug 5, 2020
Member

As the CI shows: https://travis-ci.org/github/ruby/ruby/jobs/715089657
The specs need to pass on older versions of Ruby too.

So you need to use ruby_version_is ""..."2.8" do and ruby_version_is "2.8" do guards.

I think a convenient way in this case is to define a local variable at the top of the file (or an @ivar in a before block) like:

sep = "@"
ruby_version_is "2.8" do
  sep = " "
end

and use it in the expected result.

it 'returns a String showing the event and thread for :thread_begin event' do
inspect = nil
thread_inspection = nil
TracePoint.new(:thread_begin) { |tp|

This comment has been minimized.

@eregon

eregon Aug 5, 2020
Member

Can you use a check similar to next unless TracePointSpec.target_thread? but for thread here?
Otherwise the TracePoint might catch other Threads which is unwanted as it would fail the spec randomly.
I'm thinking

thread = nil
TracePoint.new(:thread_begin) { |tp|
  next unless Thread.current == thread
it 'returns a String showing the event and thread for :thread_end event' do
inspect = nil
thread_inspection = nil
TracePoint.new(:thread_end) { |tp|

This comment has been minimized.

@eregon

eregon Aug 5, 2020
Member

Same here, needs a check for the thread

@eregon
eregon approved these changes Aug 5, 2020
@nguyenquangminh0711
Copy link
Contributor Author

@nguyenquangminh0711 nguyenquangminh0711 commented Aug 6, 2020

It looks like all the checks are green now. Thanks for your detailed reviews and great suggestions @eregon ❤️

@ko1 ko1 merged commit 1819652 into ruby:master Aug 6, 2020
100 checks passed
100 checks passed
CodeQL-Build CodeQL-Build
Details
gcc-10
Details
make (check, --jit)
Details
make (check)
Details
check_branch
Details
make (check, ubuntu-20.04)
Details
make (test, windows-2019, 2019)
Details
make (check)
Details
gcc-9
Details
make (check, --jit-wait)
Details
make (check, ubuntu-20.04, -DRUBY_DEBUG)
Details
make (test-bundler-parallel)
Details
gcc-8
Details
make (check, ubuntu-18.04)
Details
make (test-bundled-gems)
Details
gcc-7
Details
make (check, ubuntu-18.04, -DRUBY_DEBUG)
Details
make (leaked-globals)
Details
gcc-6
Details
make (check, ubuntu-16.04)
Details
gcc-5
Details
make (test-bundler-parallel, ubuntu-20.04)
Details
gcc-4.8
Details
make (test-bundler-parallel, ubuntu-20.04, -DRUBY_DEBUG)
Details
clang-11
Details
make (test-bundler-parallel, ubuntu-18.04)
Details
clang-10
Details
make (test-bundler-parallel, ubuntu-18.04, -DRUBY_DEBUG)
Details
clang-9
Details
make (test-bundled-gems, ubuntu-20.04)
Details
clang-8
Details
make (test-bundled-gems, ubuntu-20.04, -DRUBY_DEBUG)
Details
clang-7
Details
make (test-bundled-gems, ubuntu-18.04)
Details
clang-6.0
Details
make (test-bundled-gems, ubuntu-18.04, -DRUBY_DEBUG)
Details
clang-5.0
Details
make (test-all TESTS=--repeat-count=2, ubuntu-20.04)
Details
clang-4.0
Details
make (test-all TESTS=--repeat-count=2, ubuntu-18.04)
Details
clang-3.9
Details
make (leaked-globals, ubuntu-20.04)
Details
c99
Details
make (leaked-globals, ubuntu-18.04)
Details
c11
Details
c17
Details
c2x
Details
c++98
Details
c++11
Details
c++14
Details
c++17
Details
c++2a
Details
-O0
Details
-O3
Details
gmp
Details
jemalloc
Details
valgrind
Details
coroutine=ucontext
Details
coroutine=copy
Details
disable-mathn
Details
disable-jit-support
Details
disable-dln
Details
disable-rubygems
Details
OPT_THREADED_CODE=1
Details
OPT_THREADED_CODE=2
Details
OPT_THREADED_CODE=3
Details
NDEBUG
Details
RUBY_DEBUG
Details
ARRAY_DEBUG
Details
BIGNUM_DEBUG
Details
CCAN_LIST_DEBUG
Details
CPDEBUG=-1
Details
ENC_DEBUG
Details
GC_DEBUG
Details
HASH_DEBUG
Details
ID_TABLE_DEBUG
Details
RGENGC_DEBUG=-1
Details
SYMBOL_DEBUG
Details
THREAD_DEBUG=-1
Details
RGENGC_CHECK_MODE
Details
TRANSIENT_HEAP_CHECK_MODE
Details
VM_CHECK_MODE
Details
USE_EMBED_CI=0
Details
USE_FLONUM=0
Details
USE_LAZY_LOAD
Details
USE_RINCGC=0
Details
USE_SYMBOL_GC=0
Details
USE_THREAD_CACHE=0
Details
USE_TRANSIENT_HEAP=0
Details
USE_RUBY_DEBUG_LOG=1
Details
DEBUG_FIND_TIME_NUMGUESS
Details
DEBUG_INTEGER_PACK
Details
ENABLE_PATH_CHECK
Details
GC_DEBUG_STRESS_TO_CLASS
Details
GC_ENABLE_LAZY_SWEEP=0
Details
GC_PROFILE_DETAIL_MEMOTY
Details
GC_PROFILE_MORE_DETAIL
Details
CALC_EXACT_MALLOC_SIZE
Details
MALLOC_ALLOCATED_SIZE_CHECK
Details
IBF_ISEQ_ENABLE_LOCAL_BUFFER
Details
@nguyenquangminh0711 nguyenquangminh0711 deleted the nguyenquangminh0711:remove-at-prefixes-from-tracepoint branch Aug 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.