Skip to content

Fix CI infrastructure: update Ruby matrix, fix Python pip failure #382

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

Merged
ianks merged 1 commit into from
Mar 25, 2026
Merged

Fix CI infrastructure: update Ruby matrix, fix Python pip failure #382

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
10 changes: 5 additions & 5 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ jobs:
fail-fast: false
matrix:
ruby:
- '2.7'
- '3.0'
- '3.1'
- '3.2'
- '3.3.0'
- '3.3'
- '3.4'
- '4.0'
- 'truffleruby'
steps:
- uses: actions/checkout@v2
Expand All @@ -45,7 +45,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python: [ '3.7', '3.8', '3.9', '3.10', '3.11' ]
python: [ '3.8', '3.9', '3.10', '3.11' ]

services:
redis:
Expand All @@ -60,7 +60,7 @@ jobs:
sudo add-apt-repository -y ppa:deadsnakes/ppa
sudo apt-get -qq update
sudo apt-get install -y python${{ matrix.python }} python${{ matrix.python }}-distutils
sudo pip install autopep8
sudo pip install autopep8 || true
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

sudo pip install autopep8 || true masks installation failures and can hide real environment problems. Since make test no longer runs autopep8, consider removing this step entirely; otherwise, prefer installing with the intended interpreter (python -m pip ...) and letting the step fail if the dependency is required.

Suggested change
sudo pip install autopep8 || true

Copilot uses AI. Check for mistakes.
- name: Run Python tests
run: |
bin/before-install
Expand Down
1 change: 0 additions & 1 deletion bin/before-install
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ end
case ENV['SUITE']
when 'python'
Dir.chdir('python/') do
run('sudo', 'pip', 'install', '-U', 'pip')
run('sudo', 'pip', 'install', 'setuptools==68.0.0')
run('pip', '--version')
run('sudo', 'make', 'install')
Expand Down
2 changes: 1 addition & 1 deletion python/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ autolint: autopep8 lint
run_tests: clean
tox -e ${TOX_ENV} -- --durations=10 -vv tests

test: autopep8 run_tests
test: run_tests
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

Now that make test only runs tox, running it without setting TOX_ENV will use the Makefile default (which still includes py37). Since CI dropped Python 3.7, consider updating the default TOX_ENV list to match the supported matrix to avoid surprising local failures.

Copilot uses AI. Check for mistakes.

install:
pip install --ignore-installed six -e .[test]
Expand Down
17 changes: 14 additions & 3 deletions python/ciqueue/distributed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,14 @@ def shutdown(self):
def acknowledge(self, test):
return self._eval_script(
'acknowledge',
keys=[self.key('running'), self.key('processed'), self.key('owners')],
args=[test],
keys=[
self.key('running'),
self.key('processed'),
self.key('owners'),
self.key('error-reports'),
self.key('requeued-by'),
],
args=[test, test, '', 0],
) == 1

def requeue(self, test, offset=42):
Expand All @@ -107,8 +113,10 @@ def requeue(self, test, offset=42):
self.key('running'),
self.key('worker', self.worker_id, 'queue'),
self.key('owners'),
self.key('error-reports'),
self.key('requeued-by'),
],
args=[self.max_requeues, self.global_max_requeues, test, offset],
args=[self.max_requeues, self.global_max_requeues, test, test, offset, 0],
) == 1

def retry_queue(self):
Expand Down Expand Up @@ -173,9 +181,12 @@ def _try_to_reserve_test(self):
self.key('processed'),
self.key('worker', self.worker_id, 'queue'),
self.key('owners'),
self.key('requeued-by'),
self.key('workers'),
],
Comment on lines 183 to 186
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

The reserve.lua logic now depends on build:<id>:workers being accurate (it uses SCARD to decide whether a worker can immediately pick up its own requeued tests). In the Python implementation, workers are added to this set but never expired/cleaned up, so stale worker IDs from prior runs with the same build_id can change behavior and add avoidable delays. Consider expiring the workers set (and possibly requeued-by) similarly to the Ruby implementation, or deleting it when the build finishes.

Copilot uses AI. Check for mistakes.
args=[
time.time(),
42,
],
)

Expand Down
4 changes: 2 additions & 2 deletions python/tests/test_pytest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def change_test_dir(request, monkeypatch):


def expected_messages(output):
assert '= 4 failed, 2 passed, 1 skipped, 1 xpassed, 6 errors in' in output, output
assert re.search(r'= 4 failed, 2 passed, 1 skipped, 1 xpassed, (1 warning, )?6 errors in', output), output
assert re.search(r':\d+: skipping test message', output) is not None, \
"did not find 'skipping test message' in output"

Expand Down Expand Up @@ -64,7 +64,7 @@ def test_retries_and_junit_xml(self, tmpdir):
.format(queue, xml_file, filename)

output = check_output(cmd.format(queue, filename))
assert '= 4 failed, 2 passed, 4 skipped, 1 xpassed, 6 errors in' in output, output
assert re.search(r'= 4 failed, 2 passed, 4 skipped, 1 xpassed, (1 warning, )?6 errors in', output), output
assert ('integrations/pytest/test_all.py:27: skipping test message' in output
or 'integrations/pytest/test_all.py:28: skipping test message' in output), output
assert ' WILL_RETRY ' in output, output
Expand Down
2 changes: 1 addition & 1 deletion ruby/.ruby-version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.3.0
4.0
104 changes: 58 additions & 46 deletions ruby/Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,103 +7,115 @@ PATH
GEM
remote: https://rubygems.org/
specs:
activesupport (7.1.3.2)
activesupport (8.1.3)
base64
bigdecimal
concurrent-ruby (~> 1.0, >= 1.0.2)
concurrent-ruby (~> 1.0, >= 1.3.1)
connection_pool (>= 2.2.5)
drb
i18n (>= 1.6, < 2)
json
logger (>= 1.4.2)
minitest (>= 5.1)
mutex_m
tzinfo (~> 2.0)
securerandom (>= 0.3)
tzinfo (~> 2.0, >= 2.0.5)
uri (>= 0.13.1)
ansi (1.5.0)
ast (2.4.2)
base64 (0.2.0)
bigdecimal (3.1.7)
builder (3.2.4)
concurrent-ruby (1.2.3)
connection_pool (2.4.1)
diff-lcs (1.5.1)
docile (1.4.0)
drb (2.2.1)
i18n (1.14.4)
ast (2.4.3)
base64 (0.3.0)
benchmark (0.5.0)
bigdecimal (4.0.1)
builder (3.3.0)
concurrent-ruby (1.3.6)
connection_pool (3.0.2)
diff-lcs (1.6.2)
docile (1.4.1)
drb (2.2.3)
i18n (1.14.8)
concurrent-ruby (~> 1.0)
json (2.7.1)
language_server-protocol (3.17.0.3)
logger (1.6.1)
minitest (5.22.3)
minitest-reporters (1.6.1)
json (2.19.3)
language_server-protocol (3.17.0.5)
lint_roller (1.1.0)
logger (1.7.0)
minitest (5.27.0)
minitest-reporters (1.7.1)
ansi
builder
minitest (>= 5.0)
ruby-progressbar
msgpack (1.7.2)
mutex_m (0.2.0)
parallel (1.24.0)
parser (3.3.0.5)
msgpack (1.8.0)
parallel (1.27.0)
parser (3.3.10.2)
ast (~> 2.4.1)
racc
racc (1.7.3)
prism (1.9.0)
racc (1.8.1)
rainbow (3.1.1)
rake (13.1.0)
redis (5.1.0)
redis-client (>= 0.17.0)
redis-client (0.21.1)
rake (13.3.1)
redis (5.4.1)
redis-client (>= 0.22.0)
redis-client (0.28.0)
connection_pool
regexp_parser (2.9.0)
rexml (3.3.8)
rspec (3.13.0)
regexp_parser (2.11.3)
rexml (3.4.4)
rspec (3.13.2)
rspec-core (~> 3.13.0)
rspec-expectations (~> 3.13.0)
rspec-mocks (~> 3.13.0)
rspec-core (3.13.0)
rspec-core (3.13.6)
rspec-support (~> 3.13.0)
rspec-expectations (3.13.0)
rspec-expectations (3.13.5)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
rspec-mocks (3.13.0)
rspec-mocks (3.13.8)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
rspec-support (3.13.1)
rubocop (1.62.1)
rspec-support (3.13.7)
rubocop (1.86.0)
json (~> 2.3)
language_server-protocol (>= 3.17.0)
language_server-protocol (~> 3.17.0.2)
lint_roller (~> 1.1.0)
parallel (~> 1.10)
parser (>= 3.3.0.2)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.8, < 3.0)
rexml (>= 3.2.5, < 4.0)
rubocop-ast (>= 1.31.1, < 2.0)
regexp_parser (>= 2.9.3, < 3.0)
rubocop-ast (>= 1.49.0, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 2.4.0, < 3.0)
rubocop-ast (1.31.2)
parser (>= 3.3.0.4)
unicode-display_width (>= 2.4.0, < 4.0)
rubocop-ast (1.49.1)
parser (>= 3.3.7.2)
prism (~> 1.7)
ruby-progressbar (1.13.0)
securerandom (0.4.1)
simplecov (0.22.0)
docile (~> 1.1)
simplecov-html (~> 0.11)
simplecov_json_formatter (~> 0.1)
simplecov-html (0.12.3)
simplecov-html (0.13.2)
simplecov_json_formatter (0.1.4)
snappy (0.4.0)
snappy (0.5.1)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
unicode-display_width (2.5.0)
unicode-display_width (3.2.0)
unicode-emoji (~> 4.1)
unicode-emoji (4.2.0)
uri (1.1.1)

PLATFORMS
arm64-darwin-23
ruby

DEPENDENCIES
activesupport
benchmark
bundler
ci-queue!
minitest (~> 5.11)
minitest-reporters (~> 1.1)
msgpack
rake
redis
rexml
rspec (~> 3.10)
rubocop
simplecov (~> 0.12)
Expand Down
4 changes: 3 additions & 1 deletion ruby/ci-queue.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Gem::Specification.new do |spec|
spec.homepage = 'https://github.com/Shopify/ci-queue'
spec.license = 'MIT'

spec.required_ruby_version = '>= 2.7'
spec.required_ruby_version = '>= 3.1'

spec.files = lua_scripts + `git ls-files -z`.split("\x0").reject do |f|
f.match(%r{^(test|spec|features)/})
Expand All @@ -43,5 +43,7 @@ Gem::Specification.new do |spec|

spec.add_development_dependency 'snappy'
spec.add_development_dependency 'msgpack'
spec.add_development_dependency 'benchmark'
spec.add_development_dependency 'rexml'
Comment on lines +46 to +47
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

rexml is required by the library at runtime (e.g., require 'rexml/document' in the JUnit reporter and configuration helpers). Declaring it as a development dependency means gem consumers on Ruby versions where REXML is no longer bundled may hit a LoadError. Consider making rexml a runtime dependency (add_dependency) instead of (or in addition to) add_development_dependency.

Copilot uses AI. Check for mistakes.
spec.add_development_dependency 'rubocop'
end
2 changes: 1 addition & 1 deletion ruby/lib/minitest/queue/test_data.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def error_location(exception)
@error_location ||= begin
last_before_assertion = ''
backtrace_for(exception).reverse_each do |s|
break if s =~ /in .(assert|refute|flunk|pass|fail|raise|must|wont)/
break if s =~ /in [`'](?:[\w:]*[#.])?(assert|refute|flunk|pass|fail|raise|must|wont)/

last_before_assertion = s
end
Expand Down
12 changes: 6 additions & 6 deletions ruby/test/integration/grind_redis_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_grind_command_success
all tests passed every time, grinding did not uncover any flakiness
EOS
assert_equal expected.strip, output
assert_empty err
assert_empty filter_deprecation_warnings(err)
end

def test_grind_command_runs_tests
Expand Down Expand Up @@ -91,7 +91,7 @@ def test_grind_command_runs_tests
\tExpected false to be truthy.
\t test/dummy_test.rb:18:in `test_flaky'
EOS
assert_empty err
assert_empty filter_deprecation_warnings(err)
assert_equal expected.strip, output
end

Expand Down Expand Up @@ -149,7 +149,7 @@ def test_grind_max_time

run_count = runs_line.scan(/\w+/).last.to_i

assert_empty err
assert_empty filter_deprecation_warnings(err)
assert run_count < grind_count
end

Expand Down Expand Up @@ -205,7 +205,7 @@ def test_can_grind_multiple_things
\t test/dummy_test.rb:10:in `test_bar'
EOS
assert_equal expected.strip, output
assert_empty err
assert_empty filter_deprecation_warnings(err)
end

def test_grind_max_test_duration_passing
Expand Down Expand Up @@ -248,7 +248,7 @@ def test_grind_max_test_duration_passing
The 50th of test execution time is within 1000.0 milliseconds.
EOS
assert_equal expected.strip, output
assert_empty err
assert_empty filter_deprecation_warnings(err)
end

def test_grind_max_test_duration_failing
Expand Down Expand Up @@ -293,7 +293,7 @@ def test_grind_max_test_duration_failing
test_flaky_passes:
EOS
assert output.start_with?(expected.strip)
assert_empty err
assert_empty filter_deprecation_warnings(err)
end

def test_grind_max_test_duration_percentile_outside_range
Expand Down
Loading
Loading