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.

Sign up for GitHub

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 class name in reports #104

Closed
wants to merge 1 commit into from
Closed

Fix class name in reports #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/testup/file_reporter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ def report

def record(result)
super
io.puts "%s#%s = %.2f s = %s" % [result.class,
unless instance_variable_defined? :@has_klass
Copy link
Member

Choose a reason for hiding this comment

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

The logic in these two files are duplicated. I think it would be ideal if this was put into a Mix-in module that could reuse the logic across both reporters. And along with a comment that explains why this is being done the way it is.

Copy link
Contributor Author

@MSP-Greg MSP-Greg Mar 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'll be looking at some Minitest stuff later. Minitest 5.11.3 (current) does not have a Ruby version constraint, and I believe this PR is due to testup-2 using an old version (not 100% sure).

Should we/you just update the Minitest version used (and skip the reporter changes)?

Copy link
Member

Choose a reason for hiding this comment

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

I got an issue logged to look into the Minitest version. I'm spinning up on TestUp2 work, so I'll get back to this shortly.

I don't recall why a newer version of MiniTest didn't work previously. I might have never found the real cause of it. So will have to see how the latest version fares.

@has_klass = result.instance_variable_defined? :@klass
end
io.puts "%s#%s = %.2f s = %s" % [@has_klass ? result.klass : result.class,
result.name,
result.time,
result.result_code]
Expand Down
5 changes: 4 additions & 1 deletion src/testup/reporter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ def record(result)
private

def process_results(result)
unless instance_variable_defined? :@has_klass
@has_klass = result.instance_variable_defined? :@klass
end
{
:testname => "#{result.class.name}##{result.name}",
:testname => "#{@has_klass ? result.klass : result.class}##{result.name}",
:time => result.time,
:skipped => result.skipped?,
:error => result.error?,
Expand Down