diff --git a/CHANGELOG.md b/CHANGELOG.md index d72bfc42..a3f2b31e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,32 @@ * TODO +### 0.10.0 + +* Add new environment variable `KNAPSACK_PRO_FIXED_TEST_SUITE_SPLIT`. The default value is true. + + It means when you run test suite again for the same commit hash and total number of nodes and for the same branch + then you will get exactly the same test suite split. + This is the new default behavior for the knapsack_pro gem. Thanks to that when tests on one of your node failed + you can retry the node with exactly the same subset of tests that were run on the node in the first place. + + There is one edge case. When you run tests for the first time and there is no data collected about time execution of your tests then + we need to collect data to prepare the first test suite split. The second run of your tests will have fixed test suite split. + To compare if all your CI nodes are running based on the same test suite split seed you can check the value for seed in knapsack logging message + before your test starts. The message looks like: + + [knapsack_pro] Test suite split seed: 8a606431-02a1-4766-9878-0ea42a07ad21 + + * Show test suite split seed in logger based on `build_distribution_id` from Knapsack Pro API. + * Send `fixed_test_suite_split` param to build distribution Knapsack Pro API endpoint. + + Related issues: + + * https://github.com/KnapsackPro/knapsack_pro-ruby/issues/15 + * https://github.com/KnapsackPro/knapsack_pro-ruby/issues/12 + +https://github.com/KnapsackPro/knapsack_pro-ruby/compare/v0.9.0...v0.10.0 + ### 0.9.0 * Add https support for Knapsack Pro API endpoint diff --git a/README.md b/README.md index b241428b..ceb0bef4 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,7 @@ For instance when you will run tests with rake knapsack_pro:rspec then: - [When you set global variable `KNAPSACK_PRO_REPOSITORY_ADAPTER=git` (required when CI provider is not supported)](#when-you-set-global-variable-knapsack_pro_repository_adaptergit-required-when-ci-provider-is-not-supported) - [Extra configuration for CI server](#extra-configuration-for-ci-server) - [Info about ENV variables](#info-about-env-variables) + - [KNAPSACK_PRO_FIXED_TEST_SUITE_SPLITE (test suite split based on seed)](#knapsack_pro_fixed_test_suite_splite-test-suite-split-based-on-seed) - [Environment variables for debugging gem](#environment-variables-for-debugging-gem) - [Passing arguments to rake task](#passing-arguments-to-rake-task) - [Passing arguments to rspec](#passing-arguments-to-rspec) @@ -268,6 +269,38 @@ In case when you use other CI provider for instance [Jenkins](https://jenkins-ci `KNAPSACK_PRO_CI_NODE_INDEX` - index of current CI node starts from 0. Second CI node should have `KNAPSACK_PRO_CI_NODE_INDEX=1`. +#### KNAPSACK_PRO_FIXED_TEST_SUITE_SPLITE (test suite split based on seed) + +* `KNAPSACK_PRO_FIXED_TEST_SUITE_SPLIT=true` (default) + + It means when you run test suite again for the same commit hash and total number of nodes and for the same branch + then you will get exactly the same test suite split. + + Thanks to that when tests on one of your node failed you can retry the node with exactly the same subset of tests that were run on the node in the first place. + + There is one edge case. When you run tests for the first time and there is no data collected about time execution of your tests then + we need to collect data to prepare the first test suite split. The second run of your tests will have fixed test suite split. + + To compare if all your CI nodes are running based on the same test suite split seed you can check the value for seed in knapsack logging message + before your test starts. The message looks like: + + [knapsack_pro] Test suite split seed: 8a606431-02a1-4766-9878-0ea42a07ad21 + +* `KNAPSACK_PRO_FIXED_TEST_SUITE_SPLIT=false` + + When you disable fixed test suite split then your will get test suite split based on most up to date data about your test suite time execution. + For instance, when you run tests for the second time for the same commit hash then your will get more optimal test suite split than it was on the first run. + + Don't disable fixed test suite split when: + + * you expect to run the same subset of test suite multiple times for the same node (for instance your would like to retry only single CI node that failed) + + Example of issue: https://github.com/KnapsackPro/knapsack_pro-ruby/issues/15 + + * you start your tests not at the same time across your CI nodes. For instance, one of the CI node finished faster than the other CI node started. This would change the seed for the second CI node that started later. + + Example of issue: https://github.com/KnapsackPro/knapsack_pro-ruby/issues/12 + #### Environment variables for debugging gem `KNAPSACK_PRO_ENDPOINT` - Default value is `http://api.knapsackpro.com` which is endpoint for [Knapsack Pro API](http://docs.knapsackpro.com). diff --git a/lib/knapsack_pro/client/api/v1/build_distributions.rb b/lib/knapsack_pro/client/api/v1/build_distributions.rb index 85070879..6caf07de 100644 --- a/lib/knapsack_pro/client/api/v1/build_distributions.rb +++ b/lib/knapsack_pro/client/api/v1/build_distributions.rb @@ -9,6 +9,7 @@ def subset(args) endpoint_path: '/v1/build_distributions/subset', http_method: :post, request_hash: { + :fixed_test_suite_split => KnapsackPro::Config::Env.fixed_test_suite_split, :commit_hash => args.fetch(:commit_hash), :branch => args.fetch(:branch), :node_total => args.fetch(:node_total), diff --git a/lib/knapsack_pro/client/connection.rb b/lib/knapsack_pro/client/connection.rb index 83758097..42591d80 100644 --- a/lib/knapsack_pro/client/connection.rb +++ b/lib/knapsack_pro/client/connection.rb @@ -65,6 +65,15 @@ def parse_response(body) nil end + def seed + return if @response.nil? || @response == '' + response['build_distribution_id'] + end + + def has_seed? + !seed.nil? + end + def post uri = URI.parse(endpoint_url) http = Net::HTTP.new(uri.host, uri.port) @@ -78,6 +87,7 @@ def post request_uuid = http_response.header['X-Request-Id'] logger.info("API request UUID: #{request_uuid}") + logger.info("Test suite split seed: #{seed}") if has_seed? logger.info('API response:') if errors? logger.error(response) diff --git a/lib/knapsack_pro/config/env.rb b/lib/knapsack_pro/config/env.rb index b63beac3..b6eadf17 100644 --- a/lib/knapsack_pro/config/env.rb +++ b/lib/knapsack_pro/config/env.rb @@ -61,6 +61,10 @@ def endpoint end end + def fixed_test_suite_split + ENV.fetch('KNAPSACK_PRO_FIXED_TEST_SUITE_SPLIT', true) + end + def test_suite_token required_env('KNAPSACK_PRO_TEST_SUITE_TOKEN') end diff --git a/spec/fixtures/vcr_cassettes/api/v1/build_distributions/subset/success.yml b/spec/fixtures/vcr_cassettes/api/v1/build_distributions/subset/success.yml index abe937d2..f376886a 100644 --- a/spec/fixtures/vcr_cassettes/api/v1/build_distributions/subset/success.yml +++ b/spec/fixtures/vcr_cassettes/api/v1/build_distributions/subset/success.yml @@ -29,24 +29,24 @@ http_interactions: Content-Type: - application/json; charset=utf-8 Etag: - - W/"2348448eb68a360747d21fe1bb304d5b" + - W/"bba747f5e635a765e120718b7876d174" Cache-Control: - max-age=0, private, must-revalidate X-Request-Id: - - 66e07e14-0dfd-43f9-b331-2a930141dfef + - 190e26fb-676b-4b85-9737-8a90d0cce310 X-Runtime: - - '0.158064' + - '0.639590' Server: - WEBrick/1.3.1 (Ruby/2.2.3/2015-08-18) Date: - - Fri, 23 Oct 2015 20:12:23 GMT + - Tue, 28 Jun 2016 20:40:35 GMT Content-Length: - - '74' + - '137' Connection: - Keep-Alive body: encoding: UTF-8 - string: '{"node_index":1,"test_files":[{"path":"b_spec.rb","time_execution":null}]}' + string: '{"build_distribution_id":"1f905f45-c203-40f1-9891-d9a91dd11245","node_index":1,"test_files":[{"path":"b_spec.rb","time_execution":null}]}' http_version: - recorded_at: Fri, 23 Oct 2015 20:12:23 GMT + recorded_at: Tue, 28 Jun 2016 20:40:35 GMT recorded_with: VCR 2.9.3 diff --git a/spec/knapsack_pro/client/api/v1/build_distributions_spec.rb b/spec/knapsack_pro/client/api/v1/build_distributions_spec.rb index b4802a13..042568d3 100644 --- a/spec/knapsack_pro/client/api/v1/build_distributions_spec.rb +++ b/spec/knapsack_pro/client/api/v1/build_distributions_spec.rb @@ -1,5 +1,6 @@ describe KnapsackPro::Client::API::V1::BuildDistributions do describe '.subset' do + let(:fixed_test_suite_split) { double } let(:commit_hash) { double } let(:branch) { double } let(:node_total) { double } @@ -16,12 +17,17 @@ ) end + before do + expect(KnapsackPro::Config::Env).to receive(:fixed_test_suite_split).and_return(fixed_test_suite_split) + end + it do action = double expect(KnapsackPro::Client::API::Action).to receive(:new).with({ endpoint_path: '/v1/build_distributions/subset', http_method: :post, request_hash: { + fixed_test_suite_split: fixed_test_suite_split, commit_hash: commit_hash, branch: branch, node_total: node_total, diff --git a/spec/knapsack_pro/client/connection_spec.rb b/spec/knapsack_pro/client/connection_spec.rb index 2c2871d5..f21ab544 100644 --- a/spec/knapsack_pro/client/connection_spec.rb +++ b/spec/knapsack_pro/client/connection_spec.rb @@ -40,15 +40,17 @@ "{\"fake\":\"hash\",\"test_suite_token\":\"3fa64859337f6e56409d49f865d13fd7\"}", { "Content-Type" => "application/json", "Accept" => "application/json" } ).and_return(http_response) - - expect(KnapsackPro).to receive(:logger).exactly(3).and_return(logger) - expect(logger).to receive(:info).with('API request UUID: fake-uuid') - expect(logger).to receive(:info).with('API response:') end context 'when body response is json' do let(:body) { '{"errors": "value"}' } + before do + expect(KnapsackPro).to receive(:logger).exactly(3).and_return(logger) + expect(logger).to receive(:info).with('API request UUID: fake-uuid') + expect(logger).to receive(:info).with('API response:') + end + it do parsed_response = { 'errors' => 'value' } @@ -60,9 +62,36 @@ end end + context 'when body response is json with build_distribution_id' do + let(:body) { '{"build_distribution_id": "seed-uuid"}' } + + before do + expect(KnapsackPro).to receive(:logger).exactly(4).and_return(logger) + expect(logger).to receive(:info).with('API request UUID: fake-uuid') + expect(logger).to receive(:info).with("Test suite split seed: seed-uuid") + expect(logger).to receive(:info).with('API response:') + end + + it do + parsed_response = { 'build_distribution_id' => 'seed-uuid' } + + expect(logger).to receive(:info).with(parsed_response) + + expect(subject).to eq(parsed_response) + expect(connection.success?).to be true + expect(connection.errors?).to be false + end + end + context 'when body response is empty' do let(:body) { '' } + before do + expect(KnapsackPro).to receive(:logger).exactly(3).and_return(logger) + expect(logger).to receive(:info).with('API request UUID: fake-uuid') + expect(logger).to receive(:info).with('API response:') + end + it do expect(logger).to receive(:info).with('') diff --git a/spec/knapsack_pro/config/env_spec.rb b/spec/knapsack_pro/config/env_spec.rb index 938e4197..fa071cc0 100644 --- a/spec/knapsack_pro/config/env_spec.rb +++ b/spec/knapsack_pro/config/env_spec.rb @@ -219,6 +219,20 @@ end end + describe '.fixed_test_suite_split' do + subject { described_class.fixed_test_suite_split } + + context 'when ENV exists' do + before { stub_const("ENV", { 'KNAPSACK_PRO_FIXED_TEST_SUITE_SPLIT' => false }) } + it { should eq false } + end + + context "when ENV doesn't exist" do + before { stub_const("ENV", {}) } + it { should be true } + end + end + describe '.test_suite_token' do subject { described_class.test_suite_token }