Skip to content

Rewrite test framework, expand coverage, and fix isolation bugs - #133

Merged
mikecovlee merged 30 commits into
masterfrom
improve_0821
Aug 23, 2026
Merged

mikecovlee merged 30 commits into
masterfrom
improve_0821

Conversation

@mikecovlee

@mikecovlee mikecovlee commented Aug 21, 2026

Copy link
Copy Markdown
Member

This pull request updates documentation and CI workflows to reflect recent changes in resource ownership, context lifetime, and testing procedures for Covariant Script. The most important changes are:

Documentation: Ownership Model & ABI Migration

  • Clarifies that all script objects hold raw pointers to their defining context; using any escaped script object after its context is destroyed is now explicitly undefined behavior, not guaranteed to throw an exception. This affects both English (docs/SDK.md) and Chinese (docs/SDK-zh.md) SDK docs. [1] [2] [3] [4]
  • Adds a migration guide for ABI 2609xx → 2610xx, highlighting breaking changes such as the removal of function_ptr ownership, changes in structure ownership, and the shift from exceptions to UB for escaped object use after context destruction. [1] [2]
  • Updates resource ownership tables and explanations to match the new model (e.g., process and token arena ownership). [1] [2]

Testing & CI Workflow

  • Reworks GitHub Actions workflow: unit tests now run with --repeat=2 --shuffle --xml=unit_test_report.xml for both Unix and Windows, and integration tests are unified under run_tests.sh/run_tests.ps1 scripts. Adds sanitizer jobs (ASan/UBSan) for Linux.
  • Uploads unit test reports as artifacts for all platforms.

Contributor Documentation

  • Updates CONTRIBUTING.md and CONTRIBUTING-zh.md to document new test scripts, unit test options, and helper APIs. Adds instructions for generating expected outputs and writing unit tests. [1] [2]

Other Minor Improvements

  • Removes outdated references to garbage collection and clarifies the use of context and process in threading and subcontext scenarios. [1] [2] [3]

These changes ensure the documentation and CI reflect the new resource management model, clarify undefined behavior, and improve test coverage and reporting.

Framework:
- Replace test_harness.hpp with covariant_test.hpp (807 lines)
- EXPECT/ASSERT macros, EXPECT_THROW_MSG, TRACE, --timeout
- --filter/--repeat/--shuffle/--xml/--list CLI options
- JUnit XML output for CI integration

New tests:
- test_type_ext.cpp: 67 tests (String, Array, Hash_map, Hash_set, Pair, Numeric)
- test_codegen.cpp: 27 tests (constant folding, arithmetic, boolean, edge cases)
- test_system.cpp: 19 tests (file, path, env, runtime, system.run)

Bug fixes:
- Fix gc_program_arena_reclaimed_on_release isolation (make_context -> create_context)
- Fix UTF-8 encoding in test_helpers.hpp comment
- Add RAII cleanup for file/path tests

CI:
- Add --repeat=2 --shuffle to unit test commands
- Expand integration test suite with output verification
- Add --generate mode for expected output files

Documentation:
- Update CONTRIBUTING.md with unit test guide
- Add shared_compiler_context() with safety constraints
- Add plan.md for test infrastructure improvements
- sources/system/unix/common.cpp: __APPLE__/__FreeBSD__ -> COVSCRIPT_PLATFORM_DARWIN/COVSCRIPT_PLATFORM_FREEBSD
- sources/system/common.cpp: _WIN32 -> COVSCRIPT_PLATFORM_WIN32
- unit_tests/test_system.cpp: _WIN32 -> COVSCRIPT_PLATFORM_WIN32
- function::mContext, struct_builder::mContext: weak_ptr -> context_type*
- function::get_context(): return raw pointer instead of shared_ptr
- Remove resolve_ctx() and process_context::teardown_ctx entirely
- function::call_* (4 paths): use mContext directly, drop lock/throw
- struct_builder::do_inherit/operator(): use mContext directly
- fiber_function::context: weak_ptr -> raw, drop lock/throw
- fiber::create: remove context-alive check
- fiber cs_context (win32/unix): weak_ptr -> raw, remove resume expired check
- Delete 4 tests that relied on 'context destroyed' clean-failure:
  fiber_resume_rejected, escaped_function_call_rejected,
  escaped_function_fiber_create_rejected, escaped_type_constructor_rejected
- Update structure escape test comment (method invocation now UB)
- Update SDK docs: all escaped callable invocation = undefined behavior
- struct_builder: remove m_process (shared_ptr<process_context>) entirely;
  type_node lifetime now relies on context holding the process
- structure: change m_process from shared_ptr to raw context_type*;
  finalizer activation still works because context must be alive when
  finalize runs (script function needs ctx->instance)
- Update test: escaped_structure_data_usable → escaped_structure_member_data;
  remove type_node assertions (now UB after context death), keep member data
- Update SDK docs: structure member data remains valid (self-contained domain),
  but type identity is UB after context death
- Section 1 preamble: remove 'APIs backed by weak references' (all
  weak_ptr removed in ee45862)
- Section 1 footer: remove 'type identity nodes' from safe-escape list
  (process pin removed in 3ca83a3)
- Ownership table: token arena owned by 'instance, functions, and
  struct builders' (not just function store)
…egfault

cb10bfe moved function ownership from shared_ptr to unique_ptr+store,
but left struct methods registering via std::move in run_impl. Since
struct_builder::operator() re-runs method statements for every new
instance, the second instance saw a null mFunc and crashed.

Fix: register named functions and struct methods into the function_store
at compile time (codegen), matching how lambdas already work. The
statement holds a raw function* pointer into the store; run_impl only
binds the callable. This eliminates the repeated-move problem and
aligns the registration semantics for all callable types.

Verified: 335/335 unit tests, 69/69 integration tests (was 65 pass + 4
segfault), ASan clean on Linux.
Remove the 'structure member data remains valid' exception from the
escape contract. All escaped objects are now uniformly documented as
UB after context destruction, with no self-contained subset promised
to survive.

- SDK.md/SDK-zh.md: remove 'member data remains valid' from §1, §4
- Remove test escaped_structure_member_data_usable_after_context_death
  (tested the removed guarantee)
- Fix stale test comment 'callable's owner' → 'function store'
- Update plan.md to match new uniform contract

Verified: 334/334 unit tests, 69/69 integration tests.
Rewrite machine-translated phrasing in SDK-zh.md to match the project
author's preferred style: direct, concrete, no double-negative boilerplate.
Sync SDK.md to match.

Key rewrites:
- '裸非拥有指针,受 context 生命周期约束' → '指向所属 context 的裸指针,一旦销毁即失效'
- '不保证仍可用' → '一律不可再用' / 'no longer usable'
- '保活机制' → '保持存活机制'
- §1 title: remove '(escaped objects)' parenthetical
- Remove redundant '没有垃圾回收器' parenthetical
- Remove '结果是:每项资源...精确回收' paragraph (redundant with ownership model)
Document the breaking changes introduced by this branch:
- function_ptr owner removal
- contains_callable removal
- escape behavior UB (replaces runtime_error)
- get_context() / m_process raw pointer changes
- named function compile-time registration
- cs::invoke new API
…management and update related logic

test: add tests to ensure finalizers are skipped safely when context is dead
- Remove 6 interactive stdin scripts (choice/hash_map/import/optimize/
  recursion/test_coroutine) from the automatic integration list; files stay
  in tests/ for manual runs
- Per-test timeout: run_tests.sh uses 'timeout' when available; Windows uses
  run_tests.ps1 (raw Process API, WaitForExit+Kill; Start-Process' ExitCode
  is unreliable in Windows PowerShell 5.1)
- Enable expected-output comparison: 59 expected files committed, skip_output
  extended to platform/timing-dependent scripts, CRLF normalization on both
  platforms, drop run_tests.bat in favor of the self-contained run_tests.ps1
- New sanitizers CI job (ASan+UBSan) on ubuntu-22.04 for unit and integration
  tests
- CONTRIBUTING: point Windows test instructions at run_tests.ps1; fix 14
  em-dash mojibake occurrences in CONTRIBUTING.md
- covariant_test.hpp: unique XML testcase names with --repeat, --list honors
  --filter, TEST_F runs TearDown when SetUp throws
- test_system.cpp: redirect system.run echo output per platform
- .gitignore: ignore unit test artifacts; stop tracking plan.md (internal
  planning doc)
dlopen with RTLD_DEEPBIND makes the process exit silently under ASan
(google/sanitizers#611), so the extension-loading unit tests would abort
the run before the summary line. The sanitizers CI job now excludes them
with --exclude=extension_load.
matches_filter with an empty pattern returns true, so the exclude check
rejected all tests when --exclude was unset.
GCC 12's libasan misreports a stack-buffer-overflow in the sigaltstack
interceptor when a no-return path (std::exit from system.exit) executes on
a fiber (ucontext) stack that was never registered with ASan. GCC 13 does
not exhibit this. The main build matrix keeps ubuntu-22.04 for GCC 12
coverage of normal builds.
- test_set.csc: sort collected items before printing (hash iteration order
  depends on the std::hash implementation, libstdc++ vs libc++)
- to_string.csc: use type() (cs::ostream, consistent across platforms)
  instead of typeid to_string (C++ demangled name); print hash_map entries
  via sorted keys() and hash_set via sorted items
- limit.csc / numeric.csc: long double extremes depend on the architecture
  (x86-64 80-bit, aarch64 128-bit, macOS arm64 64-bit double) - move to
  skip_output (exit code only) and drop their expected files
std::ios_base::seekdir/openmode are typedef int in the MSVC STL but
distinct enum types in libstdc++, so type() output differs (int vs
cs::iostream::seekdir). Enum type names are a compiler STL detail; the
remaining 18 type() assertions are verified identical on GCC, MSVC and
Clang.

This comment was marked as resolved.

- covariant_test.hpp: include <type_traits> and <utility> explicitly
  (has_stream_op relied on transitive includes)
- run_tests.sh: capture the pipeline exit status inside the command
  substitution - PIPESTATUS is empty after $(...), so exit-code checks
  (including the 124 timeout) were silently disabled
- test_system.cpp: file_exist_true removes stale files first and asserts
  fopen success, preventing false passes and deleting pre-existing files
- SDK.md/SDK-zh.md: migration guide now says structure::m_process is
  std::weak_ptr<process_context> (liveness probe, not a raw pointer)

Verified: 674/674 unit tests and 63/63 integration tests on GCC
(Linux/MSYS2), MSVC and Clang; runner now detects non-zero exits.

This comment was marked as resolved.

…eanup

- domain_manager: add create_snapshot/restore_snapshot; compile() and REPL
  restore the snapshot on compile-time failure, so constants bound during
  preprocess no longer dangle after rollback. Runtime failures still keep
  bindings.
- drop check_binding_shape (superseded by snapshot rollback)
- import/source_import: cascade-erase transitive module keys on failure,
  fixing dangling function_ptrs in the shared module cache
- structure: hold a raw context_type* m_ctx (drop m_process); run_finalize
  recognizes object_method script finalizers and guards null m_ctx

Verified: 686/686 unit tests (--repeat=2 --shuffle) on Windows/MSYS2 ucrt64

This comment was marked as resolved.

…ents

- Remove no-op --exclude=serial_execution from sanitizer job (env auto-detect already covers it)
- Add if-no-files-found: ignore to unit test report upload
- Delete contains_callable migration-guide bullet (never shipped, cited non-existent APIs)
- Correct structure finalize: instance -> process; add m_ctx==nullptr skip note
- Add non-atomic refcount caveat to English SDK docs (already in Chinese)
- Update stale arena<->function cycle comments in impl.hpp and test_fixes.cpp
- Remove is_script_callable detection from run_finalize; guard on m_ctx
  instead of script/native distinction (same behavior, -15 lines)
- Add storage_transaction RAII class (runtime.hpp) with commit/rollback/discard
- Replace m_snap/m_committed single-slot members with m_tx vector stack,
  fixing re-entrant exec() snapshot overwrite (parallel to m_units)
- compile() reuses storage_transaction; eliminates manual snapshot lifecycle
- Update SDK docs: unified finalize behavior (script/native both scoped
  under context process; skipped with diagnostic when unavailable)
- Verified: 686 unit tests + 63 integration tests pass
@mikecovlee
mikecovlee merged commit 39ad1a5 into master Aug 23, 2026
8 checks passed
@mikecovlee
mikecovlee deleted the improve_0821 branch August 23, 2026 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants