-
Notifications
You must be signed in to change notification settings - Fork 62
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
Fix method scope in test in order to invoke the tests properly and fix exception message #182
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,9 +1,13 @@ | ||||||
# frozen_string_literal: false | ||||||
require 'test/unit' | ||||||
require 'rexml/document' | ||||||
require "test/unit" | ||||||
require "core_assertions" | ||||||
|
||||||
require "rexml/document" | ||||||
|
||||||
module REXMLTests | ||||||
class TestParseEntityDeclaration < Test::Unit::TestCase | ||||||
include Test::Unit::CoreAssertions | ||||||
|
||||||
private | ||||||
def xml(internal_subset) | ||||||
<<-XML | ||||||
|
@@ -18,25 +22,26 @@ def parse(internal_subset) | |||||
REXML::Document.new(xml(internal_subset)).doctype | ||||||
end | ||||||
|
||||||
public | ||||||
def test_empty | ||||||
exception = assert_raise(REXML::ParseException) do | ||||||
parse(<<-INTERNAL_SUBSET) | ||||||
<!ENTITY> | ||||||
INTERNAL_SUBSET | ||||||
end | ||||||
assert_equal(<<-DETAIL.chomp, exception.to_s) | ||||||
Malformed notation declaration: name is missing | ||||||
Malformed entity declaration | ||||||
Line: 5 | ||||||
Position: 72 | ||||||
Position: 70 | ||||||
Last 80 unconsumed characters: | ||||||
<!ENTITY> ]> <r/> | ||||||
> ]> <r/> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nits] There is a unnecessary white space.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, the exception message contains trailing space. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh I see. I'm so sorry mm |
||||||
DETAIL | ||||||
end | ||||||
|
||||||
def test_linear_performance_gt | ||||||
seq = [10000, 50000, 100000, 150000, 200000] | ||||||
assert_linear_performance(seq, rehearsal: 10) do |n| | ||||||
REXML::Document.new('<!DOCTYPE rubynet [<!ENTITY rbconfig.ruby_version "' + '>' * n + '">') | ||||||
REXML::Document.new('<!DOCTYPE rubynet [<!ENTITY rbconfig.ruby_version "' + '>' * n + '">]>') | ||||||
end | ||||||
end | ||||||
end | ||||||
|
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.
Could you use
match_data
notscanner
?We want to use
MatchData
instead ofStringScanner
because we don't want to useStringScanner
inREXML::Source
directly. ButREXML::Source#match
returnsStringScanner
directly for performance reason.So we should assume that the return value of
REXML::Source#match
is aMatchData
and we should only useMatchData
compatible API provided byStringScanner
.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.
I see, so that's the intention.
563bb57