-
Notifications
You must be signed in to change notification settings - Fork 116
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
add to_ascii(string) function #353
base: master
Are you sure you want to change the base?
Conversation
79bbff6
to
53b6a51
Compare
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.
Thanks @marekklis, looking good! Just need a test for the happy path and a bit of tidy up of the failure case test you've already got there.
@@ -153,6 +153,21 @@ object FunctionDefSpec extends PostgresRunnableSpec with ShopSchema { | |||
r <- testResult.runCollect | |||
} yield assert(r.head)(equalTo(expected)) | |||
|
|||
assertion.mapErrorCause(cause => Cause.stackless(cause.untraced)) | |||
}, | |||
testM("to_ascii") { |
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.
Can you make this into a suite and add a test for both failure and success cases? e.g. ToAscii("Hello") should work but this isn't tested at the moment.
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.
I can't write a success case test because our test DB default encoding is UTF-8 but to_ascii("hello")
fails with the following error:
org.postgresql.util.PSQLException: ERROR: encoding conversion from UTF8 to ASCII not supported
From postgresql docs:
Conversion is only supported from LATIN1, LATIN2, LATIN9, and WIN1250 encodings
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.
Right, sorry I misread your original comment. I'll add this to another existing issue so we can get you a shiny new container with the right encoding for this test, but this will do for now.
|
||
val testResult = execute(query).to[String, String](identity) | ||
|
||
val assertion = testResult.runCollect.fold( |
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.
you should be able to use fails
instead of fold
to get rid of assert(true)(isFalse)
something like:
val assertion = assert(testResult.runCollect.mapError(_.getMessage))(fails(equalTo("ERROR: encoding conversion from UTF8 to ASCII not supported")))
there are good examples of this in the main zio repository tests
@robmwalsh Thanks for your review. I'll address your feedback shortly |
#201