Skip to content

Commit b773128

Browse files
st0012st0012
committed
Rename ZJIT make targets for clarity and consistency
This commit improves the naming convention of ZJIT-related make targets to better communicate their purpose and create a more logical hierarchy. Renamed targets: - zjit-test → zjit-test-rust (clarifies it runs Rust unit tests) - zjit-test-lldb → zjit-test-rust-lldb (debug variant of Rust tests) - zjit-test-all → zjit-test-suite (runs the complete test suite) - zjit-test-all-ruby → zjit-test-ruby-all (better reflects intent to run all Ruby tests) Added targets: - zjit-test-ruby (runs only test/ruby/test_zjit.rb) The new naming scheme follows a clear pattern: - zjit-test-rust* for Rust-based tests - zjit-test-ruby* for Ruby-based tests - zjit-test-suite for the full test suite Also updated: - CI workflows to use the new target names - Documentation - Comments in zjit/build.rs
1 parent 3cf9622 commit b773128

5 files changed

Lines changed: 29 additions & 24 deletions

File tree

‎.github/workflows/zjit-macos.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ jobs:
3232
fail-fast: false
3333
matrix:
3434
include:
35-
- test_task: 'zjit-test'
35+
- test_task: 'zjit-test-rust'
3636
configure: '--enable-yjit=dev --enable-zjit'
3737

3838
- test_task: 'ruby' # build test for combo build
@@ -83,7 +83,7 @@ jobs:
8383
- uses: taiki-e/install-action@v2
8484
with:
8585
tool: nextest@0.9
86-
if: ${{ matrix.test_task == 'zjit-test' }}
86+
if: ${{ matrix.test_task == 'zjit-test-rust' }}
8787

8888
- name: Install Rust # TODO(alan): remove when GitHub images catch up past 1.85.0
8989
run: rustup default 1.85.0

‎.github/workflows/zjit-ubuntu.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737
configure: '--enable-zjit=dev --with-gcc=clang-14'
3838
libclang_path: '/usr/lib/llvm-14/lib/libclang.so.1'
3939

40-
- test_task: 'zjit-test'
40+
- test_task: 'zjit-test-rust'
4141
configure: '--enable-yjit --enable-zjit=dev'
4242

4343
- test_task: 'test-all'
@@ -84,7 +84,7 @@ jobs:
8484
- uses: taiki-e/install-action@v2
8585
with:
8686
tool: nextest@0.9
87-
if: ${{ matrix.test_task == 'zjit-test' }}
87+
if: ${{ matrix.test_task == 'zjit-test-rust' }}
8888

8989

9090
- uses: ./.github/actions/setup/directories

‎doc/zjit.md‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,41 +28,41 @@ in a way that can be easily shared with other team members.
2828

2929
Make sure you have a `--enable-zjit=dev` build, and run `brew install cargo-nextest` first.
3030

31-
### make zjit-test-all
31+
### make zjit-test-suite
3232

33-
This command runs all ZJIT tests: `make zjit-test` and `test/ruby/test_zjit.rb`.
33+
This command runs all ZJIT tests: `make zjit-test-rust` and `test/ruby/test_zjit.rb`.
3434

3535
```
36-
make zjit-test-all
36+
make zjit-test-suite
3737
```
3838

39-
### make zjit-test
39+
### make zjit-test-rust
4040

4141
This command runs Rust unit tests.
4242

4343
```
44-
make zjit-test
44+
make zjit-test-rust
4545
```
4646

4747
You can also run a single test case by specifying the function name:
4848

4949
```
50-
make zjit-test ZJIT_TESTS=test_putobject
50+
make zjit-test-rust ZJIT_TESTS=test_putobject
5151
```
5252

5353
If you expect that your changes cause tests to fail and they do, you can have
5454
`expect-test` fix the expected value for you by putting `UPDATE_EXPECT=1`
5555
before your test command, like so:
5656

5757
```
58-
UPDATE_EXPECT=1 make zjit-test ZJIT_TESTS=test_putobject
58+
UPDATE_EXPECT=1 make zjit-test-rust ZJIT_TESTS=test_putobject
5959
```
6060

6161
Test changes will be reviewed alongside code changes.
6262

6363
<details>
6464

65-
<summary>Setting up zjit-test</summary>
65+
<summary>Setting up zjit-test-rust</summary>
6666

6767
ZJIT uses `cargo-nextest` for Rust unit tests instead of `cargo test`.
6868
`cargo-nextest` runs each test in its own process, which is valuable since
@@ -72,7 +72,7 @@ to <https://nexte.st/docs/installation/pre-built-binaries/> for installation
7272
instructions.
7373

7474
Since it uses Cargo, you'll also need a `configure --enable-zjit=dev ...` build
75-
for `make zjit-test`. Since the tests need to link against CRuby, directly
75+
for `make zjit-test-rust`. Since the tests need to link against CRuby, directly
7676
calling `cargo test`, or `cargo nextest` likely won't build. Make sure to
7777
use `make`.
7878

‎zjit/build.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// This build script is only used for `make zjit-test` for building
1+
// This build script is only used for `make zjit-test-rust` for building
22
// the test binary; ruby builds don't use this.
33
fn main() {
44
use std::env;

‎zjit/zjit.mk‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,22 +120,27 @@ ZJIT_ALL_RUBY_TESTS := $(wildcard $(top_srcdir)/test/ruby/*.rb $(top_srcdir)/tes
120120
# Filter out excluded tests
121121
ZJIT_RUBY_TESTS := $(filter-out $(addprefix $(top_srcdir)/,$(ZJIT_EXCLUDED_TESTS)),$(ZJIT_ALL_RUBY_TESTS))
122122

123-
# Run all Ruby tests with ZJIT enabled, excluding known failing tests
124-
.PHONY: zjit-test-all-ruby
125-
zjit-test-all-ruby:
123+
# Run all Ruby tests with ZJIT enabled (temporarily excluding known failing tests)
124+
.PHONY: zjit-test-ruby-all
125+
zjit-test-ruby-all:
126126
$(MAKE) test-all TESTS='$(ZJIT_RUBY_TESTS)'
127127

128-
# Gives quick feedback about ZJIT. Not a replacement for a full test run.
129-
.PHONY: zjit-test-all
130-
zjit-test-all:
131-
$(MAKE) zjit-test
128+
# Run only the ZJIT-specific Ruby tests
129+
.PHONY: zjit-test-ruby
130+
zjit-test-ruby:
132131
$(MAKE) test-all TESTS='$(top_srcdir)/test/ruby/test_zjit.rb'
132+
133+
# Gives quick feedback about ZJIT. Not a replacement for a full test run.
134+
.PHONY: zjit-test-suite
135+
zjit-test-suite:
136+
$(MAKE) zjit-test-rust
137+
$(MAKE) zjit-test-ruby
133138
ZJIT_BINDGEN_DIFF_OPTS =
134139

135140
# Generate Rust bindings. See source for details.
136141
# Needs `./configure --enable-zjit=dev` and Clang.
137142
ifneq ($(strip $(CARGO)),) # if configure found Cargo
138-
.PHONY: zjit-bindgen zjit-bindgen-show-unused zjit-test zjit-test-lldb
143+
.PHONY: zjit-bindgen zjit-bindgen-show-unused zjit-test-rust zjit-test-rust-lldb
139144
zjit-bindgen: zjit.$(OBJEXT)
140145
ZJIT_SRC_ROOT_PATH='$(top_srcdir)' BINDGEN_JIT_NAME=zjit $(CARGO) run --manifest-path '$(top_srcdir)/zjit/bindgen/Cargo.toml' -- $(CFLAGS) $(XCFLAGS) $(CPPFLAGS)
141146
$(Q) if [ 'x$(HAVE_GIT)' = xyes ]; then $(GIT) -C "$(top_srcdir)" diff $(ZJIT_BINDGEN_DIFF_OPTS) zjit/src/cruby_bindings.inc.rs; fi
@@ -146,14 +151,14 @@ zjit-bindgen: zjit.$(OBJEXT)
146151
#
147152
# On darwin, it's available through `brew install cargo-nextest`. See
148153
# https://nexte.st/docs/installation/pre-built-binaries/ otherwise.
149-
zjit-test: libminiruby.a
154+
zjit-test-rust: libminiruby.a
150155
RUBY_BUILD_DIR='$(TOP_BUILD_DIR)' \
151156
RUBY_LD_FLAGS='$(LDFLAGS) $(XLDFLAGS) $(MAINLIBS)' \
152157
CARGO_TARGET_DIR='$(CARGO_TARGET_DIR)' \
153158
$(CARGO) nextest run --manifest-path '$(top_srcdir)/zjit/Cargo.toml' $(ZJIT_TESTS)
154159

155160
# Run a ZJIT test written with Rust #[test] under LLDB
156-
zjit-test-lldb: libminiruby.a
161+
zjit-test-rust-lldb: libminiruby.a
157162
$(Q)set -eu; \
158163
if [ -z '$(ZJIT_TESTS)' ]; then \
159164
echo "Please pass a ZJIT_TESTS=... filter to make."; \

0 commit comments

Comments
 (0)