The GitHub Actions job "npm_and_yarn in /javascript for brace-expansion - Update #1298905329" on fory.git/main has failed. Run started by GitHub user dependabot[bot] (triggered by dependabot[bot]).
Head commit for run: 14f9f4d6aef5ef75c248c19e0e9de671b95a8c0f / Sebastian Mandrean <[email protected]> fix(java): preserve ConcurrentSkipListSet comparator on copy (#3520) ## Why? As a follow-up to #3342 and #3344, I wanted to make sure there is test coverage for all the public constructors of the different Map/Set classes supported by the serializers. That exposed a copy-path bug in `ConcurrentSkipListSetSerializer` (reported as #3519) which this PR addresses. The deserialize path already preserves the comparator, but `fory.copy(...)` rebuilt `ConcurrentSkipListSet` with the default constructor path, which dropped the comparator and silently changed ordering semantics. This is easy to miss because `Set.equals(...)` still passes even when iteration order changes. ## What does this PR do? Its kind-of two part: fix the bug, and add more test coverage. Specifically: - Preserve the comparator when copying `ConcurrentSkipListSet` by overriding the collection copy construction path in `ConcurrentSkipListSetSerializer`. - Add constructor-matrix coverage for: - `TreeSet` - `TreeMap` - `ConcurrentSkipListSet` - `ConcurrentSkipListMap` - `PriorityQueue` - Exercise both round-trip deserialize and `fory.copy(...)` behavior for the relevant public constructors. - Assert concrete runtime type, comparator nullability, and observable ordering semantics. - Use queue-specific assertions for `PriorityQueue` by comparing drained poll order instead of relying on `equals(...)`. ## Related issues - Fixes #3519 - Related to #3337 - Related to #3343 ## AI Contribution Checklist - [x] Substantial AI assistance was used in this PR: `yes` / `no` - [x] If `yes`, I included a completed [AI Contribution Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs) in this PR description and the required `AI Usage Disclosure`. - [x] If `yes`, I can explain and defend all important changes without AI help. - [x] If `yes`, I reviewed AI-assisted code changes line by line before submission. - [x] If `yes`, I ran adequate human verification and recorded evidence (checks run locally or in CI, pass/fail summary, and confirmation I reviewed results). - [x] If `yes`, I added/updated tests and specs where required. - [x] If `yes`, I validated protocol/performance impacts with evidence when applicable. - [x] If `yes`, I verified licensing and provenance compliance. AI Usage Disclosure - substantial_ai_assistance: yes - scope: code drafting, test design - affected_files_or_subsystems: java/fory-core collection serializers and java/fory-core collection/map serializer tests - human_verification: local verification evidence available: `cd java && ENABLE_FORY_DEBUG_OUTPUT=1 mvn -T16 -pl fory-core -Dtest=org.apache.fory.serializer.collection.CollectionSerializersTest,org.apache.fory.serializer.collection.MapSerializersTest test` -> pass (237 tests, 0 failures) - performance_verification: no dedicated benchmark run; runtime change is limited to the `fory.copy(...)` path for `ConcurrentSkipListSet`, while serialize/deserialize hot paths are unchanged; the added comparator copy matches existing `PriorityQueue` and `ConcurrentSkipListMap` copy behavior - provenance_license_confirmation: repository-local Apache-2.0-compatible changes only; no third-party code introduced ## Does this PR introduce any user-facing change? - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark No dedicated benchmark was run. This PR is a correctness fix plus test coverage expansion, not a performance optimization: - The runtime code change only affects the `fory.copy(...)` path for `ConcurrentSkipListSet`. - Serialization and deserialization hot paths are unchanged. - The added work is one comparator copy per copied `ConcurrentSkipListSet`, which is expected to be negligible relative to element copy and skip-list insertion work, and is consistent with existing `PriorityQueue` and `ConcurrentSkipListMap` copy behavior. Co-authored-by: Shawn Yang <[email protected]> Report URL: https://github.com/apache/fory/actions/runs/23715401974 With regards, GitHub Actions via GitBox --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
