-
-
Notifications
You must be signed in to change notification settings - Fork 11
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
Expression tests and related issues #384
base: main
Are you sure you want to change the base?
Conversation
Sorry, I can't see the logs for the failing Bitrise or Test checks. I tested on macOS 14 with 5.9. (It fails as expected on Linux due to missing Foundation API's and objc support.) |
Thank you for the PR! I'll check back later. |
Sorry, please ignore the first commit to add |
Please let me know if you'd like me to rebase or merge the branch. |
No worries. I just can't take my time. |
Thank you very much. It looks good to me. All tests are passing in my local environment, so if there is a problem when I run it on CI, I will fix it on my end. |
Thank you for your patience. I rebased off main, removed the unnecessary commit, and removed the expected failures per request. fyi, four tests in AssertTests are failing for me:
(XCode 15.0, swift 5.9.0, M1 Studio) If I discover a solution for inconsistent tests, I'll make a separate PR. Thanks! |
- literals: nil, floats, int, bool, array - operators: binary, postfix, prefix, function call - optional: nil, force-unwrap, chaining - access: member, key-path
DRAFT: Need to verify test should target a keyword used as an identifier DRAFT: need to verify expected output: ``` +++ [String] `class` [-n-]{+cl+}a[-me-]{+ss+} ```
The assert macro expansion renders the inout variable constant, which makes it unusable in the expression, which causes the compile to fail. This may be a bug or limitation of the macro or swift-syntax.
This reverts commit cb6d4e2.
As promised in #299, I filled in some of the expression tests, raising three kinds of issues:
Test helper
actExp(..)
is to replace XCTest assertion directly.It identifies the first text delta and prints expected and actual cleanly for copy/paste. But it is relatively ungainly and not isolated, and there are likely diffing libraries that would do a better job. It can merge and work as-is, but I can clean it up per your preferences. I used it only in the new tests.
Three kinds of test issues
testAsyncExpression3()
: fails when there is no networktestIdentifierExprSyntax()
now refers to a quoted keyword (`class`
). Let me know if something else was intended.`class`
is rendered as[-n-]{+cl+}a[-me-]{+ss+}
. Is that correct?Two implementation issues
"Optional(\"0\")"
Merging
Commits with aspects to investigate are marked DRAFT. Commits are designed to merge even if the issues above require changes (e.g., tests should continue to pass; the current behavior is accepted for now). Commits are also separated to isolate the issues above, to allow skipping a particular issue/commit. The goal was to enable the changes to be merged, with follow-on work scheduled in later tasks.