From 0ba5f9ada6932995543307d8424e9511e0f7812d Mon Sep 17 00:00:00 2001 From: Siddhartha Date: Thu, 16 May 2024 12:12:51 -0700 Subject: [PATCH 1/3] lint: Don't re-import the same bindings from two modules --- qi-lib/flow/extended/forms.rkt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qi-lib/flow/extended/forms.rkt b/qi-lib/flow/extended/forms.rkt index a1080b8f..5f921b26 100644 --- a/qi-lib/flow/extended/forms.rkt +++ b/qi-lib/flow/extended/forms.rkt @@ -15,7 +15,8 @@ syntax/parse/define "expander.rkt" "../../macro.rkt" - "../space.rkt" + (only-in "../space.rkt" + define-for-qi) "impl.rkt") ;;; Predicates From 515122f4b2017ceeaca5764249d491cdd448b0aa Mon Sep 17 00:00:00 2001 From: Siddhartha Date: Thu, 16 May 2024 12:14:22 -0700 Subject: [PATCH 2/3] More direct "full cycle" compiler test macro --- qi-test/tests/compiler/rules/full-cycle.rkt | 22 +++++++-------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/qi-test/tests/compiler/rules/full-cycle.rkt b/qi-test/tests/compiler/rules/full-cycle.rkt index 52d21a9a..e7e17496 100644 --- a/qi-test/tests/compiler/rules/full-cycle.rkt +++ b/qi-test/tests/compiler/rules/full-cycle.rkt @@ -11,22 +11,15 @@ rackunit/text-ui syntax/macro-testing qi/flow/core/deforest + qi/flow/core/compiler "private/deforest-util.rkt" (submod qi/flow/extended/expander invoke)) (begin-for-syntax - (require syntax/parse/define - (for-template qi/flow/core/compiler) - (for-syntax racket/base)) - - ;; A macro that accepts surface syntax, expands it, and then applies the - ;; indicated optimization passes. - (define-syntax-parser test-compile~> - [(_ stx) - #'(expand-flow stx)] - [(_ stx pass ... passN) - #'(passN - (test-compile~> stx pass ...))])) + ;; A function that expands and compiles surface syntax + (define (qi-compile stx) + (compile-flow + (expand-flow stx)))) (define tests @@ -39,9 +32,8 @@ (test-true "normalize → deforest" (deforested? (phase1-eval - (test-compile~> #'(~>> (filter odd?) values (map sqr)) - normalize-pass - deforest-pass))))))) + (qi-compile + #'(~>> (filter odd?) values (map sqr))))))))) (module+ main (void From 8bff3b565ed9d52c8604236ab43579495ebb951d Mon Sep 17 00:00:00 2001 From: Siddhartha Date: Tue, 11 Jun 2024 19:20:21 -0700 Subject: [PATCH 3/3] test sandboxed eval with macro introspection This adds a regression test for the recent issue [1] where building docs for packages depending on Qi was failing due to the use of macro introspection functionality in the macro-debugger library inside a sandboxed evaluator. This was recently fixed by handling the runtime error resulting from requiring the macro debugger library (which is due to the sandboxed evaluator disallowing macro introspection). This regression test validates the bug and the fix. The coverage checker was still showing the lambda used in "monkey patching" the macro debugger utility as uncovered by tests. My guess is that this is because the coverage tool only counts code as covered if the evaluator used in the test runner evaluates those lines, whereas in the present case, the code is "covered" by a sandboxed evaluator we construct in the test rather than the test runner itself. Assuming this is the right explanation, I've placed the original fix inside a submodule---by default, the coverage checker ignores all submodules [2] for the purposes of coverage. So in cases where we are sure some code needn't be covered (or which we know is covered but isn't detected, as in this case), we can use this recipe as a standard way to add an exclusion to coverage. [1]: https://github.com/drym-org/qi/wiki/Qi-Meeting-Mar-29-2024 [2]: https://docs.racket-lang.org/cover/api.html#%28def._%28%28lib._cover%2Fmain..rkt%29._irrelevant-submodules%29%29 --- qi-lib/flow/core/passes.rkt | 11 +++++++++-- qi-test/info.rkt | 1 + qi-test/tests/flow.rkt | 25 ++++++++++++++++++++++++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/qi-lib/flow/core/passes.rkt b/qi-lib/flow/core/passes.rkt index 6bad1c35..9ee60862 100644 --- a/qi-lib/flow/core/passes.rkt +++ b/qi-lib/flow/core/passes.rkt @@ -6,7 +6,10 @@ (provide (for-syntax define-and-register-pass run-passes)) -(begin-for-syntax +(module macro-debug racket/base + ;; See tests/compiler/rules/full-cycle.rkt for an explanation + ;; re: sandboxed evaluation, submodules and test coverage. + (provide my-emit-local-step) (define my-emit-local-step ;; See "Breaking Out of the Sandbox" @@ -15,7 +18,11 @@ (make-keyword-procedure (lambda (kws kw-args . rest) (void)))))) - (dynamic-require 'macro-debugger/emit 'emit-local-step))) + (dynamic-require 'macro-debugger/emit 'emit-local-step)))) + +(require (for-syntax 'macro-debug)) + +(begin-for-syntax ;; Could be a list but for future extensibility a custom struct is ;; probably a better idea. diff --git a/qi-test/info.rkt b/qi-test/info.rkt index 2af17f8c..c788951a 100644 --- a/qi-test/info.rkt +++ b/qi-test/info.rkt @@ -6,5 +6,6 @@ (define build-deps '("rackunit-lib" "adjutor" "math-lib" + "sandbox-lib" "qi-lib")) (define clean '("compiled" "tests/compiled" "tests/private/compiled")) diff --git a/qi-test/tests/flow.rkt b/qi-test/tests/flow.rkt index df8e59c2..5218767a 100644 --- a/qi-test/tests/flow.rkt +++ b/qi-test/tests/flow.rkt @@ -11,6 +11,7 @@ racket/string racket/function racket/format + racket/sandbox (except-in "private/util.rkt" add-two) syntax/macro-testing) @@ -1657,7 +1658,29 @@ (thunk ((☯ (== _ _ _)) 3))) - (test-equal? "relay-_" ((☯ _) 3) 3)))))) + (test-equal? "relay-_" ((☯ _) 3) 3)))) + + (test-suite + "regression tests" + (test-suite + "sandboxed evaluation" + (test-not-exn "Plays well with sandboxed evaluation" + ;; See "Breaking Out of the Sandbox" + ;; https://github.com/drym-org/qi/wiki/Qi-Meeting-Mar-29-2024 + ;; + ;; This test reproduces the bug and the fix fixes it. Yet, + ;; coverage does not show the lambda in `my-emit-local-step` + ;; as being covered. This could be because the constructed + ;; sandbox evaluator "covering" the code doesn't count as + ;; coverage by the main evaluator running the test? + ;; We address this by putting `my-emit-local-step` in a + ;; submodule, which, by default, are ignored by coverage. + (lambda () + (let ([eval (make-evaluator + 'racket/base + '(require qi))]) + (eval + '(☯ add1))))))))) (module+ main (void (run-tests tests)))