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

Initialize basic tests #2

Merged
merged 6 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
name: CI
on:
push:
branches: [main]
tags: ["*"]
pull_request:
concurrency:
group: ${{ github.workflow }}-${{ github.ref || github.run_id }}
cancel-in-progress: true
jobs:
test:
name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }}
env:
JULIA_NUM_THREADS: 2
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
version:
- '1.9'
- '1.10-nightly'
os:
- ubuntu-latest
- macOS-latest
arch:
- x64
steps:
- uses: actions/[email protected]
- uses: julia-actions/setup-julia@v1
with:
version: ${{ matrix.version }}
arch: ${{ matrix.arch }}
- uses: actions/cache@v2
env:
cache-name: cache-artifacts
with:
path: ~/.julia/artifacts
key: ${{ runner.os }}-test-${{ env.cache-name }}-${{ hashFiles('**/Project.toml') }}
restore-keys: |
${{ runner.os }}-test-${{ env.cache-name }}-
${{ runner.os }}-test-
${{ runner.os }}-
- uses: julia-actions/julia-buildpkg@v1
- uses: julia-actions/julia-runtest@v1
- uses: julia-actions/julia-processcoverage@v1
- uses: codecov/codecov-action@v3
with:
file: lcov.info
45 changes: 45 additions & 0 deletions .github/workflows/CompatHelper.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
name: CompatHelper
Copy link
Member

Choose a reason for hiding this comment

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

is this worth having even if we have no deps?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just copied everything over, I thought it might be useful eventually, but can remove it if you want to

on:
schedule:
- cron: 0 0 * * *
workflow_dispatch:
permissions:
contents: write
pull-requests: write
jobs:
CompatHelper:
runs-on: ubuntu-latest
steps:
- name: Check if Julia is already available in the PATH
id: julia_in_path
run: which julia
continue-on-error: true
- name: Install Julia, but only if it is not already available in the PATH
uses: julia-actions/setup-julia@v1
with:
version: '1'
# arch: ${{ runner.arch }}
if: steps.julia_in_path.outcome != 'success'
- name: "Add the General registry via Git"
run: |
import Pkg
ENV["JULIA_PKG_SERVER"] = ""
Pkg.Registry.add("General")
shell: julia --color=yes {0}
- name: "Install CompatHelper"
run: |
import Pkg
name = "CompatHelper"
uuid = "aa819f21-2bde-4658-8897-bab36330d9b7"
version = "3"
Pkg.add(; name, uuid, version)
shell: julia --color=yes {0}
- name: "Run CompatHelper"
run: |
import CompatHelper
CompatHelper.main()
shell: julia --color=yes {0}
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
COMPATHELPER_PRIV: ${{ secrets.DOCUMENTER_KEY }}
# COMPATHELPER_PRIV: ${{ secrets.COMPATHELPER_PRIV }}
50 changes: 50 additions & 0 deletions .github/workflows/JuliaNightly.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# CI for Julia nightly, separate workflow to avoid failing CI badge on nightly fail
name: JuliaNightly
on:
push:
branches: [master]
tags: [v*]
pull_request:
schedule:
- cron: "0 0 * * *"
concurrency:
group: ${{ github.workflow }}-${{ github.ref || github.run_id }}
cancel-in-progress: true
jobs:
test:
name: ${{ matrix.os }} - ${{ matrix.arch }}
env:
JULIA_NUM_THREADS: 2
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
version:
- 'nightly'
os:
- ubuntu-latest
arch:
- x64
steps:
- uses: actions/[email protected]
- uses: julia-actions/setup-julia@v1
with:
version: ${{ matrix.version }}
arch: ${{ matrix.arch }}
- uses: actions/cache@v2
env:
cache-name: cache-artifacts
with:
path: ~/.julia/artifacts
key: ${{ runner.os }}-${{ matrix.arch }}-test-${{ env.cache-name }}-${{ hashFiles('**/Project.toml') }}
restore-keys: |
${{ runner.os }}-${{ matrix.arch }}-test-${{ env.cache-name }}-
${{ runner.os }}-${{ matrix.arch }}-test-
${{ runner.os }}-${{ matrix.arch }}-
${{ runner.os }}-
- uses: julia-actions/julia-buildpkg@v1
- uses: julia-actions/julia-runtest@v1
- uses: julia-actions/julia-processcoverage@v1
- uses: codecov/codecov-action@v1
with:
file: lcov.info
28 changes: 28 additions & 0 deletions .github/workflows/TagBot.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: TagBot
on:
issue_comment:
types:
- created
workflow_dispatch:
permissions:
actions: read
checks: read
contents: write
deployments: read
issues: read
discussions: read
packages: read
pages: read
pull-requests: read
repository-projects: read
security-events: read
statuses: read
jobs:
TagBot:
if: github.event_name == 'workflow_dispatch' || github.actor == 'JuliaTagBot'
runs-on: ubuntu-latest
steps:
- uses: JuliaRegistries/TagBot@v1
with:
ssh: ${{ secrets.DOCUMENTER_KEY }}
token: ${{ secrets.GITHUB_TOKEN }}
8 changes: 8 additions & 0 deletions Project.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
name = "ObjectStore"
uuid = "1b5eed3d-1f46-4baa-87f3-a4a892b23610"
version = "0.1.0"

[extras]
Drvi marked this conversation as resolved.
Show resolved Hide resolved
CloudBase = "85eb1798-d7c4-4918-bb13-c944d38e27ed"
ReTestItems = "817f1d60-ba6b-4fd5-9520-3cf149f6a823"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["CloudBase", "ReTestItems", "Test"]
43 changes: 22 additions & 21 deletions src/ObjectStore.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module ObjectStore

export init_rust_store, blob_get!, blob_put!, AzureCredentials
Drvi marked this conversation as resolved.
Show resolved Hide resolved

const rust_lib_dir = @static if Sys.islinux()
joinpath(
@__DIR__,
Expand Down Expand Up @@ -35,14 +37,16 @@ end

const rust_lib = joinpath(rust_lib_dir, "librust_store.$extension")

RUST_STORE_STARTED = false
const RUST_STORE_STARTED = Ref(false)
const _INIT_LOCK::ReentrantLock = ReentrantLock()
function init_rust_store()
global RUST_STORE_STARTED
if RUST_STORE_STARTED
return
Base.@lock _INIT_LOCK begin
if RUST_STORE_STARTED[]
return
end
@ccall rust_lib.start()::Cint
RUST_STORE_STARTED[] = true
end
@ccall rust_lib.start()::Cint
RUST_STORE_STARTED = true
end

struct AzureCredentials
Expand All @@ -51,23 +55,22 @@ struct AzureCredentials
key::String
host::String
end
const _AzureCredentialsFFI = NTuple{4,Cstring}

struct AzureCredentialsFFI
account::Cstring
container::Cstring
key::Cstring
host::Cstring
end
Copy link
Member

Choose a reason for hiding this comment

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

can you say why this is preferable to the existing code?

It looks more complex to me.

Copy link
Member Author

@Drvi Drvi Dec 12, 2023

Choose a reason for hiding this comment

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

Instead of manually casting struct between two types, we rely on the Julia machinery that is supposed to solve this problem: the cconvert that transforms the data into C-compatible types and followed by the unsafe_convert which can then safely create pointers from the whatever it was given by cconvert. Those two are automatically called by @ccall .

Copy link
Member Author

@Drvi Drvi Dec 12, 2023

Choose a reason for hiding this comment

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

I think this is how you are supposed to do this thing, because it handles lifetimes for you and is applicable to all @ccalls we'll do for list, head and others.

Copy link
Member Author

Choose a reason for hiding this comment

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

But if you are really against it, I'll remove it


function to_ffi(credentials::AzureCredentials)
AzureCredentialsFFI(
function Base.cconvert(::Type{Ref{AzureCredentials}}, credentials::AzureCredentials)
credentials_ffi = (
Base.unsafe_convert(Cstring, Base.cconvert(Cstring, credentials.account)),
Base.unsafe_convert(Cstring, Base.cconvert(Cstring, credentials.container)),
Base.unsafe_convert(Cstring, Base.cconvert(Cstring, credentials.key)),
Base.unsafe_convert(Cstring, Base.cconvert(Cstring, credentials.host))
)
)::_AzureCredentialsFFI
# cconvert ensures its outputs are preserved during a ccall, so we can crate a pointer
# safely in the unsafe_convert call.
return credentials_ffi, Ref(credentials_ffi)
end
function Base.unsafe_convert(::Type{Ref{AzureCredentials}}, x::Tuple{T,Ref{T}}) where {T<:_AzureCredentialsFFI}
return Base.unsafe_convert(Ptr{_AzureCredentialsFFI}, x[2])
end


struct Response
result::Cint
Expand All @@ -79,7 +82,6 @@ end

function blob_get!(path::String, buffer::AbstractVector{UInt8}, credentials::AzureCredentials)
response = Ref(Response())
credentials_ffi = Ref(to_ffi(credentials))
size = length(buffer)
cond = Base.AsyncCondition()
cond_handle = cond.handle
Expand All @@ -88,7 +90,7 @@ function blob_get!(path::String, buffer::AbstractVector{UInt8}, credentials::Azu
path::Cstring,
buffer::Ref{Cuchar},
size::Culonglong,
credentials_ffi::Ref{AzureCredentialsFFI},
credentials::Ref{AzureCredentials},
response::Ref{Response},
cond_handle::Ptr{Cvoid}
)::Cint
Expand Down Expand Up @@ -117,7 +119,6 @@ end

function blob_put!(path::String, buffer::AbstractVector{UInt8}, credentials::AzureCredentials)
response = Ref(Response())
credentials_ffi = Ref(to_ffi(credentials))
size = length(buffer)
cond = Base.AsyncCondition()
cond_handle = cond.handle
Expand All @@ -126,7 +127,7 @@ function blob_put!(path::String, buffer::AbstractVector{UInt8}, credentials::Azu
path::Cstring,
buffer::Ref{Cuchar},
size::Culonglong,
credentials_ffi::Ref{AzureCredentialsFFI},
credentials::Ref{AzureCredentials},
response::Ref{Response},
cond_handle::Ptr{Cvoid}
)::Cint
Expand Down
80 changes: 80 additions & 0 deletions test/azure_blobs_tests.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
@testitem "Basic BlobStorage tests" begin

using CloudBase.CloudTest: Azurite
using ObjectStore: blob_get!, blob_put!, AzureCredentials
import ObjectStore

ObjectStore.init_rust_store()

# For interactive testing, use Azurite.run() instead of Azurite.with()
# conf, p = Azurite.run(; debug=true); atexit(() -> kill(p))
Azurite.with(; debug=true, public=false) do conf
_credentials, _container = conf
base_url = @something(_container.baseurl, "")
Drvi marked this conversation as resolved.
Show resolved Hide resolved
credentials = AzureCredentials(_credentials.auth.account, _container.name, _credentials.auth.key, base_url)

@testset "0B file" begin
buffer = Vector{UInt8}(undef, 1000)

nbytes_written = blob_put!(joinpath(base_url, "empty.csv"), codeunits(""), credentials)
@test nbytes_written == 0

nbytes_read = blob_get!(joinpath(base_url, "empty.csv"), buffer, credentials)
@test nbytes_read == 0
end

@testset "100B file" begin
input = "1,2,3,4,5,6,7,8,9,1\n" ^ 5
buffer = Vector{UInt8}(undef, 1000)
@assert sizeof(input) == 100
@assert sizeof(buffer) > sizeof(input)

nbytes_written = blob_put!(joinpath(base_url, "test100B.csv"), codeunits(input), credentials)
@test nbytes_written == 100

nbytes_read = blob_get!(joinpath(base_url, "test100B.csv"), buffer, credentials)
@test nbytes_read == 100
@test String(buffer[1:nbytes_read]) == input
end

@testset "1MB file" begin
input = "1,2,3,4,5,6,7,8,9,1\n" ^ 50_000
buffer = Vector{UInt8}(undef, 1_000_000)
@assert sizeof(input) == 1_000_000 == sizeof(buffer)

nbytes_written = blob_put!(joinpath(base_url, "test100B.csv"), codeunits(input), credentials)
@test nbytes_written == 1_000_000

nbytes_read = blob_get!(joinpath(base_url, "test100B.csv"), buffer, credentials)
@test nbytes_read == 1_000_000
@test String(buffer[1:nbytes_read]) == input
end

@testset "20MB file" begin
input = "1,2,3,4,5,6,7,8,9,1\n" ^ 1_000_000
buffer = Vector{UInt8}(undef, 20_000_000)
@assert sizeof(input) == 20_000_000 == sizeof(buffer)

nbytes_written = blob_put!(joinpath(base_url, "test100B.csv"), codeunits(input), credentials)
@test nbytes_written == 20_000_000

nbytes_read = blob_get!(joinpath(base_url, "test100B.csv"), buffer, credentials)
@test nbytes_read == 20_000_000
@test String(buffer[1:nbytes_read]) == input
end

# If the buffer is too small, we hang
# @testset "100B file, buffer too small" begin
# input = "1,2,3,4,5,6,7,8,9,1\n" ^ 5
# buffer = Vector{UInt8}(undef, 10)
# @assert sizeof(input) == 100
# @assert sizeof(buffer) < sizeof(input)

# nbytes_written = blob_put!(joinpath(base_url, "test100B.csv"), codeunits(input), credentials)
# @test nbytes_written == 100

# nbytes_read = blob_get!(joinpath(base_url, "test100B.csv"), buffer, credentials)
# end
end # Azurite.with

end # @testitem
6 changes: 6 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
using ReTestItems
using ObjectStore

withenv("RUST_BACKTRACE"=>1) do
runtests(ObjectStore, testitem_timeout=30, nworkers=1)
end
Loading