-
Notifications
You must be signed in to change notification settings - Fork 276
Lift WebKit Building and Passing Tests in Scala 3 #2041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reduce Scala 3 compilation errors from 354 to 2 (99.4% reduction). ## Major Changes ### Scala 3 Syntax & Type System - Fix FactoryMaker vend() call syntax (vend()() vs vend.apply()) - Add explicit types to PartialFunction pattern matches - Fix lambda parameter syntax (require parentheses with type annotations) - Fix union type handling for Box[NodeSeq].openOr with explicit matching - Add explicit return types for recursive/overloaded methods - Fix indentation issues causing scope problems in Scala 3 ### Visibility & Access Modifiers - Change private[http] to public for cross-package access in Scala 3: - LiftRules: lockedSecurityRules, notFoundOrIgnore, dispatchTable - LiftRules: snippet, bootFinished, withMimeHeaders - Scala 3 has stricter rules for package-private access ### Version-Specific Code - Create scala_3/NamedCometDispatcher.scala without parallel collections - Add scala.jdk.CollectionConverters import for Scala 3 compatibility ### Type Safety Improvements - Fix Box[String] vs String comparisons using .contains() - Add explicit type annotations to createSnapshot methods - Fix toString return type declarations ### Code Structure - Fix case block brace placement for proper scoping - Correct indentation in match expressions and for-comprehensions - Fix lambda syntax in dropWhile, filter, and CSS selector transforms ## Breaking Changes - Some private[http] members are now public - NamedCometDispatcher.scala uses sequential iteration instead of parallel ## Known Issues (2 remaining errors) - Wizard.scala Field trait shadowing issue - Scala 3 doesn't allow nested traits to shadow parent traits with the same name. Requires refactoring to rename Wizard.Screen.Field or create Scala 3-specific version. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Scala 2 and Scala 3 type inference these slightly differently. Being explicit about the type here should ensure that the code cross-compiles successfully.
This doesn't compile correctly in Scala 3
Migrate lift-webkit test suite from Scala 2.13 to Scala 3.3.6. This
resolves all 397+ compilation errors by updating test code to work with
specs2 5.x and Scala 3 requirements.
Key changes:
- Update specs2 Scope usage: Move from org.specs2.specification.Scope
to org.specs2.execute.Scope and change instantiation syntax
- Fix Mockito integration: Replace specs2.mock.Mockito with
scalatestplus.mockito.MockitoSugar and update stubbing syntax
- Handle BeforeEach.before return type change: Wrap side effects in
step {} blocks to return Fragments
- Fix protected method access: Create public wrapper methods for testing
- Update matcher syntax for specs2 5.x: have length -> haveLength
- Fix lambda type inference: Add explicit parameter types
- Serialize JSON to bytes for mock requests using compact/render
- Replace MatchResult with Success/Failure pattern
- Add explicit Unit returns in step blocks
- Add success markers in multi-assertion test blocks
- Fix type ascriptions for Box types in eventually matchers
All test files now compile successfully with only deprecation warnings
(Manifest -> ClassTag transitions) remaining.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Create separate test files for Scala 2.13 and Scala 3 to handle differences between specs2 4.x and 5.x APIs. Changes: - Move 6 test files with API differences to scala-2.13/ and scala-3/ - Keep remaining 37 test files in shared scala/ directory - Tests now compile successfully in both Scala 2.13.16 and 3.3.6 Scala 2.13 (specs2 4.x) uses: - org.specs2.specification.Scope - org.specs2.mock.Mockito with "returns" syntax - result() method for custom matchers Scala 3 (specs2 5.x) uses: - org.specs2.execute.Scope - org.mockito.Mockito with when().thenReturn() syntax - Success/Failure for custom matchers Files with version-specific implementations: - SpecContextHelpers.scala (Scope location) - LiftMergeSpec.scala (Mockito syntax) - ReqSpec.scala (Scope + Mockito) - LiftJavaScriptSpec.scala (Scope location) - OfflineRequestSnapshotSpec.scala (Mockito syntax) - XMLApiSpec.scala (Matcher result format) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…3.3.6
Fixes critical test failures where all LiftMergeSpec tests were failing with
"IllegalStateException: Attempted to use resource bundles outside of an
initialized S scope".
Root Cause:
The test helper traits (WithRules, WithLiftContext) were using the Scope
pattern with empty blocks that caused ThreadLocal state to be set and
immediately cleared BEFORE test execution. This left tests without proper
Lift context (LiftRules and S scope).
The original trait-based approach relied on specs2's Around pattern which
wrapped test execution, but the migration to Scope for specs2 5.x broke
this - Scope only runs code during construction, not during test execution.
Solution:
Migrated from trait-based initialization to explicit wrapper functions
following specs2 5.x best practices for Scala 3 compatibility:
- Replaced trait-based helpers with SpecContextHelpers object
- Added withLiftRules(rules)(test) - wraps test with LiftRules context
- Added withLiftContext(rules, session)(test) - wraps test with both
LiftRules and S scope
- Properly manages ThreadLocal lifecycle during test execution using
doWith {} blocks that contain the actual test code
Changes:
- Refactored SpecContextHelpers.scala (Scala 2.13 & 3.3.6 versions)
- Updated LiftMergeSpec.scala to use new wrapper functions
- Updated HtmlFixerSpec.scala to use new wrapper functions
Results:
- All 13 LiftMergeSpec tests now pass in Scala 2.13.16 ✅
- All 13 LiftMergeSpec tests now pass in Scala 3.3.6 ✅
- Cross-version compatible test infrastructure
- No reliance on deprecated DelayedInit
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Fixes test isolation failure in LiftSessionSpec where the second test was
seeing state from the first test, causing:
Vector(1, 2, ..., 20, 1, 1) != Vector(1, 1)
Root Cause:
The BeforeEach trait's `before` method has different API requirements
between specs2 4.x (Scala 2.13) and specs2 5.x (Scala 3):
- specs2 4.x: before method executes action directly
- specs2 5.x: before method must wrap action in step()
The shared implementation used `step {}` which worked for Scala 3 but
failed silently on Scala 2.13 - the cleanup never ran, causing shared
mutable state to leak between tests.
Solution:
Split LiftSessionSpec into version-specific implementations:
- Scala 2.13: `override def before = { receivedMessages = Vector[Int]() }`
- Scala 3.3.6: `override def before = step { receivedMessages = Vector[Int]() }`
Results:
- All 4 LiftSessionSpec tests now pass in Scala 2.13.16 ✅
- All 4 LiftSessionSpec tests now pass in Scala 3.3.6 ✅
- Proper test isolation with no state leaking between examples
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Fixes two test failures in SnippetSpec: Scala 2.13.16 failures: - "Correctly process not lift:designer_friendly" (line 84) - "reflect only the last pushed values with S.currentAttrs" (line 450) Scala 3.3.6 failures: - "reflect only the last pushed values with S.currentAttrs" (line 450) Root Causes: 1. XML Comparison Using Wrong Matcher (line 84): Used strict equality (===) instead of XML-aware matcher (==/). The === operator performs object equality which is sensitive to whitespace, attribute ordering, and formatting differences in XML. All other XML tests in the file correctly use the ==/ matcher. 2. Nested Assertion Result Propagation (line 450): Test had assertions only at the innermost nesting level with no assertions as the nested S.withAttrs blocks unwound. This caused the test result to not propagate correctly through the nested function calls, particularly in specs2 4.x (Scala 2.13). Solutions: 1. Changed line 84 from `ret === xml` to `ret must ==/(xml)` to use XML-aware comparison that normalizes XML before matching. 2. Added assertions at each unwinding level of the nested S.withAttrs blocks (lines 448, 451-452, 455) to ensure proper result propagation. This follows the pattern used by the passing test "properly reflect the full snippet stack with S.attrs" (lines 409-435). Results: - All 23 SnippetSpec tests now pass in Scala 2.13.16 ✅ - All 23 SnippetSpec tests now pass in Scala 3.3.6 ✅ - 2 tests remain pending (intentionally marked as such in the code) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The Box class defines its own === operator that returns Boolean for value comparison. This shadows specs2's === matcher, causing test assertions to return boolean values instead of proper specs2 Results. Changes: - Split MemoizeSpec into version-specific files - Scala 2.13: Use mustEqual which works with specs2 4.x - Scala 3.3.6: Use must be_== to avoid Box's === method - Changed S.initIfUninitted to S.init with Full(Req.nil) for proper session context initialization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The Box class defines its own === operator that returns Boolean for value comparison. This shadows specs2's === matcher, causing test assertions to return boolean values instead of proper specs2 Results. Changes: - Split WebSpecSpec into version-specific files - Scala 2.13: Use must_=== which works with specs2 4.x - Scala 3.3.6: Use must be_== to avoid Box's === method - Mark template processing test as pending on Scala 3 due to ClassCastException with snippet instantiation (deeper Scala 3 compatibility issue that needs separate investigation) Test Results: - Scala 2.13: 13 examples, 0 failures ✅ - Scala 3.3.6: 12 passed, 1 pending ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Split LAFutureWithSessionSpec, MockWebSpec, and LocSpec into Scala 2.13 and 3.3.6 specific versions to handle Box equality matcher differences between specs2 4.x and 5.x. Changes: - Scala 2.13: Uses mustEqual and must_=== for Box/Boolean comparisons - Scala 3: Uses must be_== to avoid Box's === operator - MockWebSpec: Added missing contentType header for JSON test All tests now pass on both Scala 2.13.16 and 3.3.6. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…flection Resolves snippet class instantiation failures in Scala 3.3.6 that were causing ClassCastException when constructing snippet instances via reflection. Root Cause: Scala 3's stricter type inference would default generic type parameter T to Nothing when calling makeOne[T] without explicit type context, causing attempts to cast snippet instances to scala.runtime.Nothing$ which failed. Solution: - Changed ConstructorType.makeOne methods to return AnyRef instead of T - Added targetClass parameter to ConstructorType trait and implementations - Used Class.cast() for runtime type verification before final cast - Moved final asInstanceOf[T] to call site where type context is available - Fixed variable shadowing issue (paramClz vs clz) in constructFrom - Enhanced error handling for ClassCastException and InvocationTargetException The fix uses Class.cast() which performs proper runtime type checking, making the code more type-safe while working correctly in both Scala 2.13 and Scala 3.3.6. Tests Fixed: - OneShot: all 6 tests now pass (snippet callbacks execute correctly) - ToHeadUsages: all 12 tests now pass (snippet rendering works) - WebSpecSpec: template processing test un-pended and passing - Full test suite: 281 tests pass on Scala 3.3.6 (previously had failures) - No regressions: 282 tests pass on Scala 2.13 (unchanged) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates Lift WebKit to support Scala 3 by addressing test compatibility issues, updating type handling, and creating version-specific test files. The changes primarily focus on migrating specs2 test syntax from version 4.x (Scala 2.13) to version 5.x (Scala 3), along with necessary adjustments to reflection-based code and type handling.
Key changes include:
- Migration from
must_==to===and other specs2 matcher updates across all test files - Creation of separate test source directories for Scala 2.13 and Scala 3 to handle API differences
- Updates to reflection-based constructor invocation with explicit type casting for Scala 3 compatibility
- Dependency management changes to use version-specific specs2 and mockito libraries
Reviewed changes
Copilot reviewed 71 out of 78 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Test files (multiple) | Updated test assertion syntax from specs2 4.x to 5.x compatible format (e.g., must_== → ===) |
scala-3/ test directory |
New Scala 3-specific test files with specs2 5.x syntax and Scope instead of Around |
scala-2.13/ test directory |
New Scala 2.13-specific test files maintaining specs2 4.x compatibility |
LiftSession.scala |
Enhanced constructor reflection with explicit type casting and improved error handling |
Templates.scala |
Updated ConstructorType sealed trait with explicit targetClass field and type-safe casting |
Wizard.scala |
Added explicit return types and fixed generic type parameter references |
LiftResponse.scala |
Changed StreamingResponse.data from structural type to explicit InputStream |
Dependencies.scala |
Added version-specific dependency resolution for specs2 and mockito |
build.sbt |
Updated to use version-specific dependency functions |
| Various source files | Code formatting, indentation fixes, and explicit type annotations for Scala 3 compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
This PR makes various updates to Lift WebKit to get tests passing in Scala 3. A lot of the diff is creating additional test files in order to handle major differences in how tests need to be handled.