Merge reports feature

[jaredatron]

Are there plans to add a merge_report feature so knapsack will update the report file instead of overwriting it?

I'd be happy to put together a PR is the feature is wanted :)

[ArturT]

What's the use case? You would like to run just part of your test suite and record for it the time execution. Then merge the result with the existing report.

Currently, you generate the report with time execution for all tests on your CI with command like:

KNAPSACK_GENERATE_REPORT=true bundle exec rspec spec

and you'd like to do something like

KNAPSACK_GENERATE_REPORT=true KNAPSACK_MERGE_REPORT=true bundle exec rspec spec/models/

The output of this command will contain time execution for models specs. The existing knapsack report file will be merged with the report for models specs.

Caution: You should avoid generating a report with merge feature on your development machine and mixing it with the report generated on CI because performance on your machine and CI server are different so test suite split on CI based on merged knapsack report will be less accurate.

Is this a feature you are going to use? You can also try http://knapsackpro.com/ and get a free API key. It will keep your reports up to date on knapsackpro.com server.

I'd like to keep knapsack gem simple without too many confusing things. If you are going to prepare PR and info how to use merge report feature then I'm fine to review it. We can also wait and see if anyone would +1 this request and you can in the mean time check knapsackpro gem.

[jaredatron]

Right now we're running our build on travis across 5 parallel VMs. So we
have the knapsack report checked in so each VM has the same exact report to
ensure consistent division of labor. In order to ensure all of our specs
are in the knapsack report I've installed a pack to knapsack that amends
any rspec file times to the existing report which allows our team to check
in changes to the report along side changes to spec files.

On Thu, Apr 7, 2016 at 12:30 PM, Artur Trzop notifications@github.com
wrote:

What's the use case? You would like to run just part of your test suite
and record for it the time execution. Then merge the result with the
existing report.

Currently, you generate the report with time execution for all tests on
your CI with command like:

KNAPSACK_GENERATE_REPORT=true bundle exec rspec spec

and you'd like to do something like

KNAPSACK_GENERATE_REPORT=true KNAPSACK_MERGE_REPORT=true bundle exec rspec spec/models/

The output of this command will contain time execution for models specs.
The existing knapsack report file will be merged with the report for models
specs.

Caution: You should avoid generating a report with merge feature on
your development machine and mixing it with the report generated on CI
because performance on your machine and CI server are different so test
suite split on CI based on merged knapsack report will be less accurate.

Is this a feature you are going to use? You can also try
http://knapsackpro.com/ and get a free API key. It will keep your reports
up to date on knapsackpro.com server.

I'd like to keep knapsack gem simple without too many confusing things. If
you are going to prepare PR and info how to use merge report feature then
I'm fine to review it. We can also wait and see if anyone would +1 this
request and you can in the mean time check knapsackpro gem.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#34 (comment)

Jared

[ArturT]

I've installed a pack to knapsack that amends
any rspec file times to the existing report which allows our team to check
in changes to the report along side changes to spec files.

At first, it sounds good but the thing that bothers me is the case when a developer checks in updated knapsack report based on time execution of tests on his machine. The same specs may run different time on different developers' machines or CI servers. When test files have long time execution like feature tests with capybara then this may have a significant impact on the end test suite split result.

Do you have the pack you mentioned somewhere? Is it a fork of knapsack gem?

[jaredatron]

This is the monkey patch we're using

# Monkey Patch Knapsack to merge reports instead of overwrite
begin
  class Knapsack::Report
    alias_method :save_without_leading_existing_report, :save
    def save
      Knapsack::Presenter.existing_report = open
      save_without_leading_existing_report
    end
  end

  class << Knapsack::Presenter
    attr_writer :existing_report

    def report_hash

@existing_report.merge(Knapsack.tracker.test_files_with_time).sort.to_h
    end

    def report_yml
      report_hash.to_yaml
    end

    def report_json
      JSON.pretty_generate(report_hash)
    end
  end
end

It doesn't feel like an elegant solution so I didn't bother to PR it. But
if you think this is a feature you may wants I'd be happy to take a stab at
a sustainable code change.

:D

On Mon, Apr 11, 2016 at 2:48 PM, Artur Trzop notifications@github.com
wrote:

I've installed a pack to knapsack that amends
any rspec file times to the existing report which allows our team to check
in changes to the report along side changes to spec files.

At first, it sounds good but the thing that bothers me is the case when a
developer checks in updated knapsack report based on time execution of
tests on his machine. The same specs may run different time on different
developers' machines or CI servers. When test files have long time
execution like feature tests with capybara then this may have a significant
impact on the end test suite split result.

Do you have the pack you mentioned somewhere? Is it a fork of knapsack gem?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#34 (comment)

Jared

[ArturT]

@deadlyicon Thanks for sharing this. Maybe someone will try this. You can turn it into gist for instance.

For now, I'd like to keep it simple and don't introduce this to knapsack as this solution adds more complexity depend on the use case. Anyway, thank you for letting me know about your idea.

[jaredatron]

thank you!

feel free to share:
https://gist.github.com/deadlyicon/9c06ace46821b080dd368cc54d52b3aa

On Mon, Apr 11, 2016 at 4:08 PM, Artur Trzop notifications@github.com
wrote:

@deadlyicon https://github.com/deadlyicon Thanks for sharing this.
Maybe someone will try this. You can turn it into gist for instance.

For now, I'd like to keep it simple and don't introduce this to knapsack
as this solution adds more complexity depend on the use case. Anyway, thank
you for letting me know about your idea.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#34 (comment)

Jared

[ArturT]

I've updated FAQ in readme https://github.com/ArturT/knapsack#how-to-update-existing-knapsack-report-for-a-few-test-files
Thanks for gist!

[jaredatron]

awesome thanks!

On Mon, Apr 11, 2016 at 4:39 PM, Artur Trzop notifications@github.com
wrote:

I've updated FAQ in readme
https://github.com/ArturT/knapsack#how-to-update-existing-knapsack-report-for-a-few-test-files
Thanks for gist!

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#34 (comment)

Jared