Skip to content

Commit e29ad7c

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 bd45f4f commit e29ad7c

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
@@ -112,22 +112,27 @@ ZJIT_ALL_RUBY_TESTS := $(wildcard $(top_srcdir)/test/ruby/test_*.rb $(top_srcdir
112112
# Filter out excluded tests
113113
ZJIT_RUBY_TESTS := $(filter-out $(addprefix $(top_srcdir)/,$(ZJIT_EXCLUDED_TESTS)),$(ZJIT_ALL_RUBY_TESTS))
114114

115-
# Run all Ruby tests with ZJIT enabled, excluding known failing tests
116-
.PHONY: zjit-test-all-ruby
117-
zjit-test-all-ruby:
115+
# Run all Ruby tests with ZJIT enabled (temporarily excluding known failing tests)
116+
.PHONY: zjit-test-ruby-all
117+
zjit-test-ruby-all:
118118
$(MAKE) test-all TESTS='$(ZJIT_RUBY_TESTS)'
119119

120-
# Gives quick feedback about ZJIT. Not a replacement for a full test run.
121-
.PHONY: zjit-test-all
122-
zjit-test-all:
123-
$(MAKE) zjit-test
120+
# Run only the ZJIT-specific Ruby tests
121+
.PHONY: zjit-test-ruby
122+
zjit-test-ruby:
124123
$(MAKE) test-all TESTS='$(top_srcdir)/test/ruby/test_zjit.rb'
124+
125+
# Gives quick feedback about ZJIT. Not a replacement for a full test run.
126+
.PHONY: zjit-test-suite
127+
zjit-test-suite:
128+
$(MAKE) zjit-test-rust
129+
$(MAKE) zjit-test-ruby
125130
ZJIT_BINDGEN_DIFF_OPTS =
126131

127132
# Generate Rust bindings. See source for details.
128133
# Needs `./configure --enable-zjit=dev` and Clang.
129134
ifneq ($(strip $(CARGO)),) # if configure found Cargo
130-
.PHONY: zjit-bindgen zjit-bindgen-show-unused zjit-test zjit-test-lldb
135+
.PHONY: zjit-bindgen zjit-bindgen-show-unused zjit-test-rust zjit-test-rust-lldb
131136
zjit-bindgen: zjit.$(OBJEXT)
132137
ZJIT_SRC_ROOT_PATH='$(top_srcdir)' BINDGEN_JIT_NAME=zjit $(CARGO) run --manifest-path '$(top_srcdir)/zjit/bindgen/Cargo.toml' -- $(CFLAGS) $(XCFLAGS) $(CPPFLAGS)
133138
$(Q) if [ 'x$(HAVE_GIT)' = xyes ]; then $(GIT) -C "$(top_srcdir)" diff $(ZJIT_BINDGEN_DIFF_OPTS) zjit/src/cruby_bindings.inc.rs; fi
@@ -138,14 +143,14 @@ zjit-bindgen: zjit.$(OBJEXT)
138143
#
139144
# On darwin, it's available through `brew install cargo-nextest`. See
140145
# https://nexte.st/docs/installation/pre-built-binaries/ otherwise.
141-
zjit-test: libminiruby.a
146+
zjit-test-rust: libminiruby.a
142147
RUBY_BUILD_DIR='$(TOP_BUILD_DIR)' \
143148
RUBY_LD_FLAGS='$(LDFLAGS) $(XLDFLAGS) $(MAINLIBS)' \
144149
CARGO_TARGET_DIR='$(CARGO_TARGET_DIR)' \
145150
$(CARGO) nextest run --manifest-path '$(top_srcdir)/zjit/Cargo.toml' $(ZJIT_TESTS)
146151

147152
# Run a ZJIT test written with Rust #[test] under LLDB
148-
zjit-test-lldb: libminiruby.a
153+
zjit-test-rust-lldb: libminiruby.a
149154
$(Q)set -eu; \
150155
if [ -z '$(ZJIT_TESTS)' ]; then \
151156
echo "Please pass a ZJIT_TESTS=... filter to make."; \

0 commit comments

Comments
 (0)