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.

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

Update shard logic to evenly distribute slow tests #17

Merged
merged 1 commit into from
Feb 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion lib/mighty_test/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def run_test_by_line_number
def run_tests_by_path
test_paths = find_test_paths
test_paths = excluding_slow_paths(test_paths) unless path_args.any? || ci? || options[:all]
test_paths = Sharder.from_argv(options[:shard], env:).shard(test_paths) if options[:shard]
test_paths = Sharder.from_argv(options[:shard], env:, file_system:).shard(test_paths) if options[:shard]

run_tests_and_exit!(*test_paths)
end
Expand Down
19 changes: 15 additions & 4 deletions lib/mighty_test/sharder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@ module MightyTest
class Sharder
DEFAULT_SEED = 123_456_789

def self.from_argv(value, env: ENV)
def self.from_argv(value, env: ENV, file_system: FileSystem.new)
index, total = value.to_s.match(%r{\A(\d+)/(\d+)\z})&.captures&.map(&:to_i)
raise ArgumentError, "shard: value must be in the form INDEX/TOTAL (e.g. 2/8)" if total.nil?

git_sha = env.values_at("GITHUB_SHA", "CIRCLE_SHA1").find { |sha| !sha.to_s.strip.empty? }
seed = git_sha&.unpack1("l_")

new(index:, total:, seed:)
new(index:, total:, seed:, file_system:)
end

attr_reader :index, :total, :seed

def initialize(index:, total:, seed: nil)
def initialize(index:, total:, seed: nil, file_system: FileSystem.new)
raise ArgumentError, "shard: total shards must be a number greater than 0" unless total > 0

valid_group = index > 0 && index <= total
Expand All @@ -23,13 +23,24 @@ def initialize(index:, total:, seed: nil)
@index = index
@total = total
@seed = seed || DEFAULT_SEED
@file_system = file_system
end

def shard(*test_paths)
random = Random.new(seed)
shuffled_paths = test_paths.flatten.shuffle(random:)

# Shuffle slow and normal paths separately so that slow ones get evenly distributed
shuffled_paths = test_paths
.flatten
.partition { |path| !file_system.slow_test_path?(path) }
.flat_map { |paths| paths.shuffle(random:) }

slices = shuffled_paths.each_slice(total)
slices.filter_map { |slice| slice[index - 1] }
end

private

attr_reader :file_system
end
end
42 changes: 41 additions & 1 deletion test/mighty_test/sharder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def test_for_a_given_seed_it_generates_a_stable_shuffled_result
assert_equal(%w[f e c], result)
end

def test_it_divdes_items_into_roughly_equally_sized_shards
def test_it_divides_items_into_roughly_equally_sized_shards
all = %w[a b c d e f g h i j k l m n o p q r]
shards = (1..4).map do |index|
Sharder.new(index:, total: 4).shard(all)
Expand All @@ -67,5 +67,45 @@ def test_it_divdes_items_into_roughly_equally_sized_shards

assert_equal all, shards.flatten.sort
end

def test_it_evenly_distributes_slow_paths_across_shards
all = %w[
test/system/login_test.rb
test/system/admin_test.rb
test/models/post_test.rb
test/system/editor_test.rb
test/models/user_test.rb
test/system/email_test.rb
test/models/comment_test.rb
test/system/rss_test.rb
test/models/category_test.rb
test/system/moderation_test.rb
]
shards = (1..3).map do |index|
Sharder.new(index:, total: 3).shard(all)
end

assert_equal(
[
%w[
test/models/user_test.rb
test/models/post_test.rb
test/system/login_test.rb
test/system/admin_test.rb
],
%w[
test/models/comment_test.rb
test/system/rss_test.rb
test/system/moderation_test.rb
],
%w[
test/models/category_test.rb
test/system/email_test.rb
test/system/editor_test.rb
]
],
shards
)
end
end
end