Skip to content
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

WIP: Tests for Base.cwstring #56123

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rileysheridan
Copy link

I made simple unit tests for this function, but I would like some clarification on empty string and NUL terminated string inputs. cwstring currently throws an error for both, but I test for valid return vectors. If that's wrong, I can change the first and third tests.

Also, sorry for the wall of filemode changes. I tried to remove them from the commit but kept getting "unable to index file" errors. Advice on how to fix would be appreciated!

test/strings/basic.jl Outdated Show resolved Hide resolved
Fixes equality operator in first test of "cwstring" testset

Co-authored-by: Sukera <[email protected]>
@giordano
Copy link
Contributor

Also, sorry for the wall of filemode changes. I tried to remove them from the commit but kept getting "unable to index file" errors. Advice on how to fix would be appreciated!

What operating system are you using? How are you interacting with git? The command line interface/VS Code/something else?

@@ -1409,3 +1409,18 @@ end
@test transcode(String, transcode(UInt8, transcode(UInt16, str))) == str
end
end

@testset "cwstring" begin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base.cwstring is only defined on Windows:

if ccall(:jl_get_UNAME, Any, ()) === :NT
"""
Base.cwstring(s)
Converts a string `s` to a NUL-terminated `Vector{Cwchar_t}`, suitable for passing to C
functions expecting a `Ptr{Cwchar_t}`. The main advantage of using this over the implicit
conversion provided by [`Cwstring`](@ref) is if the function is called multiple times with the
same argument.
This is only available on Windows.
"""
function cwstring(s::AbstractString)
bytes = codeunits(String(s))
0 in bytes && throw(ArgumentError("embedded NULs are not allowed in C strings: $(repr(s))"))
return push!(transcode(UInt16, bytes), 0)
end
end
This testset should only be run on Windows, otherwise it'll fail on other platforms, which means you should change this to

if Sys.iswindows()
    @testset "cwstring" begin
        # ...
    end
end

@giordano giordano added system:windows Affects only Windows test This change adds or pertains to unit tests strings "Strings!" labels Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
strings "Strings!" system:windows Affects only Windows test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants