Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Allow --profile flag to take numeric argument #726

Closed
sferik opened this Issue · 13 comments

4 participants

@sferik

I'd like to be able to output more than the default number of examples (10) provided by the --profile flag.

My use case is: I have tried to optimize the 10 slowest examples in my suite but the same 10 examples are still the slowest. Now, I'd like to try to optimize the next 10 slowest examples.

@myronmarston

I've wanted this sometimes, too. I have another idea for how to solve it, though: config options that let you opt in or opt out particular examples from being considered for the top N slowest examples based on metadata. You may want to only profile examples tagged with :unit, for example. Or, in your case, you could tag the existing 10 slowest examples (which you can't optimize further) with something like :skip_profile so they are not considered.

I think both approaches have their merits. Thoughts?

@sferik

That sounds fine to me…but even with the ability to skip profiling, I think I'd still want the ability to display more than the 10 slowest examples, especially for larger test suites (1000s of examples).

@dchelimsky
Owner

I'd prefer a new formatter for this over making --profile more complex. i.e. --format profile (or similar).

@sferik

Maybe I'm missing something but profile output seems independent from the output format. For example, I might want both documentation format and profile output. These should not be mutually exclusive options.

@greggroth

The DocumentationFormatter inherits from BaseTextFormatter which has #dump_profile defined so they aren't mutually exclusive options. Right now, that it does the 10 slowest examples is hardcoded into BaseTextFormatter#dump_profile. @myronmarston's suggestion of a config option might be easier to implement. What do y'all think? Is this worth adding a config option for? Is there a better way?

@myronmarston

I get @dchelimsky point that a new formatter could get as fancy as you want with profiling (the formatter API is very flexible in this regard) w/o affecting any of the existing code...but I still think I'd prefer it be a config option. Plus, I think it'd be weird to have the --profile option on the one hand, and --format extended_profiler (or whatever) on the other--I'd prefer to see just one way to enable profiling from the command line.

@greggroth -- do you want to take a stab at adding a config option?

@dchelimsky
Owner

Agreed there should be one way to do it. I just think it should be a formatter that does it, deprecating the --profile option :)

At a high level, I think the simplest, most extensible design would be to do everything through formatters - even things like randomization seed, etc. This would require some though/re-design to make them truly composable, but it would, IMO, make the presentation code simpler and more flexible in the long run.

@greggroth

I don't mind taking a shot at adding a config option. Is this a reasonable short-term solution @dchelimsky?

@dchelimsky
Owner

@greggroth depends on whether @myronmarston agrees moving toward a world of composable formatters is the right way to go. If he does, then no, I think this would be a bandaid on a wart and would hurt harder later when it's deprecated. If not, then it's not only a good short-term solution, it's a good long term solution :)

@myronmarston

I'm on the fence about this: I love composable designs, but I also really like the fact that the profile output is available from multiple formatters. Moving this into a separate formatter would not easily support adding profile info to any formatter.

Overall, I agree with @dchelimsky that it would have been a better code design to put the profiling in its own formatter from the start, but given how it works now, I'm not convinced it's worth the ROI to refactor towards a separate formatter. So yeah, go ahead and take a stab at this, @greggroth. If I change my mind in the future regarding the ROI of refactoring to a separate formatter, I can deal with the added complexity of an additional config option.

@greggroth greggroth referenced this issue from a commit in greggroth/rspec-core
@greggroth greggroth add config option to specify profile example count
addresses #726
1a0accf
@myronmarston

Closing this now that #747 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.