From ed144bd52b4211722084a1f13641756e625b4d70 Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Thu, 23 Nov 2023 22:34:05 -0600 Subject: [PATCH 1/9] Enable more std tests for interpreter This is only a workaround to make forking work in interpreter It requires setting env var HANDLE_SIGNAL_IN_INTERPRETER to 1 to enable the workaround --- .github/workflows/interpreter.yml | 3 +++ spec/generate_interpreter_spec.sh | 3 +++ spec/interpreter_std_spec.cr | 20 ++++++++++---------- spec/std/signal_spec.cr | 10 ++++++---- spec/std/time/format_spec.cr | 1 + spec/std/time/time_spec.cr | 1 + src/crystal/system/unix/signal.cr | 2 +- src/kernel.cr | 7 +++++++ 8 files changed, 32 insertions(+), 15 deletions(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index a034a9f5b410..8f1e629e3f4d 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -42,8 +42,11 @@ jobs: test-interpreter-std_spec: needs: build-interpreter runs-on: ubuntu-22.04 + timeout-minutes: 15 container: image: crystallang/crystal:1.10.1-build + env: + HANDLE_SIGNAL_IN_INTERPRETER: "1" strategy: matrix: part: [0, 1, 2, 3] diff --git a/spec/generate_interpreter_spec.sh b/spec/generate_interpreter_spec.sh index f3500115a737..4e847bb53dd3 100755 --- a/spec/generate_interpreter_spec.sh +++ b/spec/generate_interpreter_spec.sh @@ -11,6 +11,9 @@ set +x export SPEC_SUITE=$1 export OUT_FILE=$2 export CRYSTAL_BIN=${CRYSTAL_BIN:-./bin/crystal} +# Enable signal handlers in interpreted code +export HANDLE_SIGNAL_IN_INTERPRETER=1 + job_size=${3:-64} command="$0 $*" diff --git a/spec/interpreter_std_spec.cr b/spec/interpreter_std_spec.cr index 6ca5aeac2ea3..0526848c5d7c 100644 --- a/spec/interpreter_std_spec.cr +++ b/spec/interpreter_std_spec.cr @@ -1,4 +1,4 @@ -# 2023-04-12 22:54:02-05:00 This file is autogenerated by `spec/generate_interpreter_spec.sh std spec/interpreter_std_spec.cr` +# 2023-11-23 20:33:13-06:00 This file is autogenerated by `spec/generate_interpreter_spec.sh std spec/interpreter_std_spec.cr 4` require "./std/array_spec.cr" require "./std/atomic_spec.cr" require "./std/base64_spec.cr" @@ -62,7 +62,7 @@ require "./std/digest/md5_spec.cr" require "./std/digest/sha1_spec.cr" require "./std/digest/sha256_spec.cr" require "./std/digest/sha512_spec.cr" -# require "./std/dir_spec.cr" (failed to run) +require "./std/dir_spec.cr" require "./std/double_spec.cr" require "./std/ecr/ecr_lexer_spec.cr" require "./std/ecr/ecr_spec.cr" @@ -70,8 +70,8 @@ require "./std/enumerable_spec.cr" require "./std/enum_spec.cr" require "./std/env_spec.cr" require "./std/errno_spec.cr" -# require "./std/exception/call_stack_spec.cr" (failed to run) -# require "./std/exception_spec.cr" (failed to run) +require "./std/exception/call_stack_spec.cr" +require "./std/exception_spec.cr" require "./std/file_spec.cr" require "./std/file/tempfile_spec.cr" require "./std/file_utils_spec.cr" @@ -114,7 +114,7 @@ require "./std/io/argf_spec.cr" require "./std/io/buffered_spec.cr" require "./std/io/byte_format_spec.cr" require "./std/io/delimited_spec.cr" -# require "./std/io/file_descriptor_spec.cr" (failed to run) +require "./std/io/file_descriptor_spec.cr" require "./std/io/hexdump_spec.cr" require "./std/io/io_spec.cr" require "./std/io/memory_spec.cr" @@ -129,7 +129,7 @@ require "./std/json/parser_spec.cr" require "./std/json/pull_parser_spec.cr" require "./std/json/serializable_spec.cr" require "./std/json/serialization_spec.cr" -# require "./std/kernel_spec.cr" (failed to run) +require "./std/kernel_spec.cr" require "./std/levenshtein_spec.cr" # require "./std/llvm/aarch64_spec.cr" (failed to run) # require "./std/llvm/arm_abi_spec.cr" (failed to run) @@ -183,7 +183,7 @@ require "./std/pointer_spec.cr" require "./std/pp_spec.cr" require "./std/pretty_print_spec.cr" require "./std/process/find_executable_spec.cr" -# require "./std/process_spec.cr" (failed to run) +require "./std/process_spec.cr" require "./std/process/status_spec.cr" require "./std/process/utils_spec.cr" require "./std/proc_spec.cr" @@ -199,7 +199,7 @@ require "./std/regex/match_data_spec.cr" require "./std/regex_spec.cr" require "./std/semantic_version_spec.cr" require "./std/set_spec.cr" -# require "./std/signal_spec.cr" (failed to run) +require "./std/signal_spec.cr" require "./std/slice_spec.cr" require "./std/socket/address_spec.cr" require "./std/socket/addrinfo_spec.cr" @@ -213,7 +213,7 @@ require "./std/spec/context_spec.cr" require "./std/spec/expectations_spec.cr" require "./std/spec/filters_spec.cr" require "./std/spec/helpers/iterate_spec.cr" -# require "./std/spec/hooks_spec.cr" (failed to run) +require "./std/spec/hooks_spec.cr" require "./std/spec/junit_formatter_spec.cr" require "./std/spec_spec.cr" require "./std/spec/tap_formatter_spec.cr" @@ -231,7 +231,7 @@ require "./std/symbol_spec.cr" # require "./std/syscall_spec.cr" (failed to run) require "./std/system_error_spec.cr" require "./std/system/group_spec.cr" -# require "./std/system_spec.cr" (failed to run) +require "./std/system_spec.cr" require "./std/system/user_spec.cr" require "./std/thread/condition_variable_spec.cr" require "./std/thread/mutex_spec.cr" diff --git a/spec/std/signal_spec.cr b/spec/std/signal_spec.cr index e92b74a6370c..2b9aac6c0f59 100644 --- a/spec/std/signal_spec.cr +++ b/spec/std/signal_spec.cr @@ -31,10 +31,12 @@ describe "Signal" do ran.should be_true end - it "ignores a signal" do - Signal::USR2.ignore - Process.signal Signal::USR2, Process.pid - end + {% unless flag?(:interpreted) %} + it "ignores a signal" do + Signal::USR2.ignore + Process.signal Signal::USR2, Process.pid + end + {% end %} it "CHLD.reset sets default Crystal child handler" do Signal::CHLD.reset diff --git a/spec/std/time/format_spec.cr b/spec/std/time/format_spec.cr index c6f8dc9b03bf..0d84749991e2 100644 --- a/spec/std/time/format_spec.cr +++ b/spec/std/time/format_spec.cr @@ -1,4 +1,5 @@ require "../spec_helper" +require "../../support/time" require "spec/helpers/string" def parse_time(format, string) diff --git a/spec/std/time/time_spec.cr b/spec/std/time/time_spec.cr index d257bc70f2d4..bfae4900705e 100644 --- a/spec/std/time/time_spec.cr +++ b/spec/std/time/time_spec.cr @@ -1,4 +1,5 @@ require "../spec_helper" +require "../../support/time" require "spec/helpers/iterate" CALENDAR_WEEK_TEST_DATA = [ diff --git a/src/crystal/system/unix/signal.cr b/src/crystal/system/unix/signal.cr index c30a2b985af2..4d3aed4d984d 100644 --- a/src/crystal/system/unix/signal.cr +++ b/src/crystal/system/unix/signal.cr @@ -56,7 +56,7 @@ module Crystal::System::Signal end end - private def self.start_loop + def self.start_loop spawn(name: "Signal Loop") do loop do value = reader.read_bytes(Int32) diff --git a/src/kernel.cr b/src/kernel.cr index c3b3106ccae3..140ae7384d12 100644 --- a/src/kernel.cr +++ b/src/kernel.cr @@ -590,3 +590,10 @@ end Crystal::Scheduler.init_workers {% end %} {% end %} + +{% if flag?(:interpreted) %} + if ENV["HANDLE_SIGNAL_IN_INTERPRETER"]? == "1" + Signal::CHLD.reset + Crystal::System::Signal.start_loop + end +{% end %} From e7d22491502b74de895732a4d145fc7bca867571 Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Thu, 23 Nov 2023 23:34:53 -0600 Subject: [PATCH 2/9] Run in 8 groups --- .github/workflows/interpreter.yml | 4 ++-- spec/interpreter_std_spec.cr | 2 +- spec/std/signal_spec.cr | 10 ++++------ 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index 8f1e629e3f4d..3badb0312d31 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -49,7 +49,7 @@ jobs: HANDLE_SIGNAL_IN_INTERPRETER: "1" strategy: matrix: - part: [0, 1, 2, 3] + part: [0, 1, 2, 3, 4, 5, 6, 7] name: "Test std_spec with interpreter (${{ matrix.part }})" steps: - uses: actions/checkout@v4 @@ -64,4 +64,4 @@ jobs: run: chmod +x .build/crystal - name: Run std_spec with interpreter - run: SPEC_SPLIT="${{ matrix.part }}%4" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${{ matrix.part }}.xml + run: SPEC_SPLIT="${{ matrix.part }}%8" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${{ matrix.part }}.xml diff --git a/spec/interpreter_std_spec.cr b/spec/interpreter_std_spec.cr index 0526848c5d7c..e90b32f7730c 100644 --- a/spec/interpreter_std_spec.cr +++ b/spec/interpreter_std_spec.cr @@ -199,7 +199,7 @@ require "./std/regex/match_data_spec.cr" require "./std/regex_spec.cr" require "./std/semantic_version_spec.cr" require "./std/set_spec.cr" -require "./std/signal_spec.cr" +# require "./std/signal_spec.cr" (failed to run) require "./std/slice_spec.cr" require "./std/socket/address_spec.cr" require "./std/socket/addrinfo_spec.cr" diff --git a/spec/std/signal_spec.cr b/spec/std/signal_spec.cr index 2b9aac6c0f59..e92b74a6370c 100644 --- a/spec/std/signal_spec.cr +++ b/spec/std/signal_spec.cr @@ -31,12 +31,10 @@ describe "Signal" do ran.should be_true end - {% unless flag?(:interpreted) %} - it "ignores a signal" do - Signal::USR2.ignore - Process.signal Signal::USR2, Process.pid - end - {% end %} + it "ignores a signal" do + Signal::USR2.ignore + Process.signal Signal::USR2, Process.pid + end it "CHLD.reset sets default Crystal child handler" do Signal::CHLD.reset From 17d0e10021475c095f0ee0022c11f76b35f718c6 Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Fri, 24 Nov 2023 00:37:34 -0600 Subject: [PATCH 3/9] Disable hooks_spec.cr --- .github/workflows/interpreter.yml | 1 + spec/interpreter_std_spec.cr | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index 3badb0312d31..6810d6e9d68f 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -49,6 +49,7 @@ jobs: HANDLE_SIGNAL_IN_INTERPRETER: "1" strategy: matrix: + fail-fast: false part: [0, 1, 2, 3, 4, 5, 6, 7] name: "Test std_spec with interpreter (${{ matrix.part }})" steps: diff --git a/spec/interpreter_std_spec.cr b/spec/interpreter_std_spec.cr index e90b32f7730c..a4c8dc1738b0 100644 --- a/spec/interpreter_std_spec.cr +++ b/spec/interpreter_std_spec.cr @@ -213,7 +213,7 @@ require "./std/spec/context_spec.cr" require "./std/spec/expectations_spec.cr" require "./std/spec/filters_spec.cr" require "./std/spec/helpers/iterate_spec.cr" -require "./std/spec/hooks_spec.cr" +# require "./std/spec/hooks_spec.cr" (failed to run) require "./std/spec/junit_formatter_spec.cr" require "./std/spec_spec.cr" require "./std/spec/tap_formatter_spec.cr" From 37b4887ca44b2602d3e8766a684aa850a4e30981 Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Fri, 24 Nov 2023 00:52:58 -0600 Subject: [PATCH 4/9] Fix workflow file --- .github/workflows/interpreter.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index 6810d6e9d68f..3badb0312d31 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -49,7 +49,6 @@ jobs: HANDLE_SIGNAL_IN_INTERPRETER: "1" strategy: matrix: - fail-fast: false part: [0, 1, 2, 3, 4, 5, 6, 7] name: "Test std_spec with interpreter (${{ matrix.part }})" steps: From 9b6ee69514f352810096f49e2ae25b4d4a64f5e5 Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Fri, 24 Nov 2023 01:52:24 -0600 Subject: [PATCH 5/9] Run tests in one container --- .github/workflows/interpreter.yml | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index 3badb0312d31..227da1d01740 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -47,10 +47,7 @@ jobs: image: crystallang/crystal:1.10.1-build env: HANDLE_SIGNAL_IN_INTERPRETER: "1" - strategy: - matrix: - part: [0, 1, 2, 3, 4, 5, 6, 7] - name: "Test std_spec with interpreter (${{ matrix.part }})" + name: "Test std_spec with interpreter" steps: - uses: actions/checkout@v4 @@ -64,4 +61,14 @@ jobs: run: chmod +x .build/crystal - name: Run std_spec with interpreter - run: SPEC_SPLIT="${{ matrix.part }}%8" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${{ matrix.part }}.xml + run: | + status=0 + for part in $(seq 0 7); do + SPEC_SPLIT="${part}%8" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${part}.xml + job_status=$? + echo $job_status + if [ $job_status -ne 0 ]; then + status = $? + fi + done + exit status From e8e4e2697acea7c62a505f2a0e36f96ebe8fd576 Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Fri, 24 Nov 2023 02:05:12 -0600 Subject: [PATCH 6/9] Update timeout config --- .github/workflows/interpreter.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index 227da1d01740..2f5f0488084e 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -42,7 +42,7 @@ jobs: test-interpreter-std_spec: needs: build-interpreter runs-on: ubuntu-22.04 - timeout-minutes: 15 + timeout-minutes: 60 container: image: crystallang/crystal:1.10.1-build env: @@ -64,7 +64,7 @@ jobs: run: | status=0 for part in $(seq 0 7); do - SPEC_SPLIT="${part}%8" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${part}.xml + timeout 900 env SPEC_SPLIT="${part}%8" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${part}.xml job_status=$? echo $job_status if [ $job_status -ne 0 ]; then From d816c5ce3f101693018bcc3ffba44bda748a2af2 Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Fri, 24 Nov 2023 02:26:43 -0600 Subject: [PATCH 7/9] Split tests into more groups --- .github/workflows/interpreter.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index 2f5f0488084e..6e546130bffc 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -42,7 +42,7 @@ jobs: test-interpreter-std_spec: needs: build-interpreter runs-on: ubuntu-22.04 - timeout-minutes: 60 + timeout-minutes: 90 container: image: crystallang/crystal:1.10.1-build env: @@ -63,12 +63,14 @@ jobs: - name: Run std_spec with interpreter run: | status=0 - for part in $(seq 0 7); do - timeout 900 env SPEC_SPLIT="${part}%8" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${part}.xml + for part in $(seq 0 15); do + echo "Running part ${part}" + timeout 900 env SPEC_SPLIT="${part}%16" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${part}.xml job_status=$? echo $job_status if [ $job_status -ne 0 ]; then - status = $? + status=$job_status fi done + echo "final status: ${status}" exit status From d46e257bcc7555502c1ff569805820e6d4ac0c94 Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Fri, 24 Nov 2023 02:42:55 -0600 Subject: [PATCH 8/9] Return status --- .github/workflows/interpreter.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index 6e546130bffc..c0b00b703862 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -62,9 +62,11 @@ jobs: - name: Run std_spec with interpreter run: | + set -e + export HANDLE_SIGNAL_IN_INTERPRETER=1 status=0 for part in $(seq 0 15); do - echo "Running part ${part}" + echo "Running part ${part} of 15" timeout 900 env SPEC_SPLIT="${part}%16" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${part}.xml job_status=$? echo $job_status @@ -73,4 +75,4 @@ jobs: fi done echo "final status: ${status}" - exit status + exit $status From 3b19d2249e5ad82d9e7d933b8fb941b85817818b Mon Sep 17 00:00:00 2001 From: Chao Yang Date: Fri, 24 Nov 2023 03:47:51 -0600 Subject: [PATCH 9/9] Redirect stderr --- .github/workflows/interpreter.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/interpreter.yml b/.github/workflows/interpreter.yml index c0b00b703862..41320ce1d74d 100644 --- a/.github/workflows/interpreter.yml +++ b/.github/workflows/interpreter.yml @@ -61,13 +61,14 @@ jobs: run: chmod +x .build/crystal - name: Run std_spec with interpreter + shell: bash run: | set -e export HANDLE_SIGNAL_IN_INTERPRETER=1 status=0 for part in $(seq 0 15); do echo "Running part ${part} of 15" - timeout 900 env SPEC_SPLIT="${part}%16" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${part}.xml + timeout 900 env SPEC_SPLIT="${part}%16" bin/crystal i spec/interpreter_std_spec.cr -- --junit_output .junit/interpreter-std_spec.${part}.xml 2> /dev/null job_status=$? echo $job_status if [ $job_status -ne 0 ]; then