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

Changes for 0.12.0 release #70

Merged
merged 13 commits into from
Nov 29, 2023
Merged

Changes for 0.12.0 release #70

merged 13 commits into from
Nov 29, 2023

Conversation

samritchie
Copy link
Collaborator

@samritchie samritchie requested a review from bartelink November 29, 2023 07:09
@samritchie
Copy link
Collaborator Author

I’ve also added the setting to resolve #68 but it didn’t seem to be working in the unit test - unsure if I’m misunderstanding what it’s supposed to do (or DynamoDB-local doesn’t support it)

@@ -104,6 +100,14 @@ type ``Conditional Expression Tests`` (fixture : TableFixture) =
fun () -> table.PutItem(item, precondition = itemDoesNotExist)
|> shouldFailwith<_, ConditionalCheckFailedException>

let [<Fact(Skip = "Not sure if this should be working")>] ``Item not exists failure should add conflicting item data to the exception`` () =
Copy link
Member

Choose a reason for hiding this comment

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

this looks reasonable to me - don't have a prod env to hand so I guess that points the finget of blame at the simulators...

RELEASE_NOTES.md Outdated
@@ -1,3 +1,10 @@
### 0.11.3-beta
Copy link
Member

Choose a reason for hiding this comment

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

A .12 might not be unreasonable as there are new supported expression features (and a removal of an obsoletion)

@samritchie samritchie changed the title Changes for 0.11.3 release Changes for 0.12.0 release Nov 29, 2023
@samritchie samritchie merged commit aabcd25 into master Nov 29, 2023
4 checks passed
let item = mkItem()
let _key = table.PutItem(item, precondition = itemDoesNotExist)
try
table.PutItem(item, precondition = itemDoesNotExist) |> ignore
Copy link
Member

Choose a reason for hiding this comment

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

@samritchie Hm, probably should assert that it does not just succeed
by replacing the try-with with a single raisesWith expression

@@ -1,27 +1,23 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
<IsPackable>false</IsPackable>
Copy link
Member

@bartelink bartelink Nov 29, 2023

Choose a reason for hiding this comment

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

I should have removed this in my PR - has not been required since SDK v 6 (MS.Test reference sets this flag by default)


type ``Inverse GSI Table Operation Tests`` (fixture : TableFixture) =

let rand = let r = Random() in fun () -> int64 <| r.Next()
Copy link
Member

Choose a reason for hiding this comment

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

.NET 7 has a Shard.Random which is threadsafe (and also means the seed can't be identical if lots of instances of the TestClass get constructed in the same timeslice)

let [<Fact>] ``Query by Table Key and GSI`` () =
let values = set [ for _ in 1L .. 1000L -> mkItem() ]
for batch in values |> Set.toSeq |> Seq.chunkBySize 25 do
table.BatchPutItems batch |> ignore
Copy link
Member

Choose a reason for hiding this comment

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

|> ignores give me kittens in general. I guess the assertions are the point here, but I'd be tempted to assign to a temp and then do a skipped =! Array.empry


let [<Fact>] ``Shared GSI Range Keys should be permitted`` () =
let template = RecordTemplate.Define<SameRangeKeyGSI>()
test <@ template.GlobalSecondaryIndices[0].RangeKey.IsSome @>
Copy link
Member

@bartelink bartelink Nov 29, 2023

Choose a reason for hiding this comment

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

For future reference, IsSome is a big clue that you should match....

test <@ match template.GlobalSecondaryIndices[0].RangeKey, template.GlobalSecondaryIndices[1].RangeKey with
        | Some r1, Some r2 -> r1 = r2
        | _ -> false @>

OR

let rk1, rk2 = trap <@ template.GlobalSecondaryIndices[0].RangeKey |> Option.get
                       template.GlobalSecondaryIndices[1].RangeKey |> Option.get @>
r1 =! r2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hash/sort keys and GSI hash/sort keys can not be set on same field
3 participants