Ms/bdd concepts #90

FrankUrbach · 2021-08-10T00:16:01Z

This is the PR for the fully implemented BDD tests. Some changes were needed to accomplish the goal. Hints, suggestions and questions are very appreciated.

…ient.jl into ms/bdd-concepts

implemented

case something whent wrong by quering something

implemented relation_type_get_relates_for_role_label and the necessary functions for this.

implemented

are worked through. The debugging is open.

graql part were deleted because they are not relevant for now.

properly

ThingType

using TypQL.

ProtoBuf files

Project.toml

startup

tk3369

This is a great PR! I don't have much to say about logic and BDDs and so I left some minor comments and formatting suggestions.

tk3369 · 2021-08-15T16:53:21Z

Project.toml

@@ -9,13 +9,19 @@ DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
 Pretend = "8ad1c615-040c-41b0-a18f-ae9e9fd09b5b"
 ProtoBuf = "3349acd9-ac6a-5e09-bcdb-63829b23a429"
+SnoopCompile = "aa65fe97-06da-5843-b5b1-d5d13cad87d2"


Is SnoopCompile needed here or in the global env?

Removed. Only a relict from working on starting time.

tk3369 · 2021-08-15T16:56:11Z

protocol_translation.jl

@@ -1,7 +1,7 @@
 using Pkg
 using ProtoBuf

-root_dir_java = "/Users/frank/Documents/JuliaProjects/JuliaSource/TypeDBClient/client-java"
+# root_dir_java = "/Users/frank/Documents/JuliaProjects/JuliaSource/TypeDBClient/client-java"


Let's just delete this line

tk3369 · 2021-08-15T16:57:42Z

src/common/rpc/RequestBuilder.jl


 function _treq(; kwargs...)
    return Proto.Transaction_Req(
        concept_manager_req = Proto.ConceptManager_Req(; kwargs...)
    )
 end

-function put_entity_type_req(label::String)
+function put_entity_type_req(label::Optional{String})


How do we handle nothing then?

Sorry. I think this was only a try to solve a problem. Changed back

tk3369 · 2021-08-15T17:02:11Z

src/common/rpc/RequestBuilder.jl

+                                            Proto.RelationType_SetRelates_Req(;
+                                                label = role_label,
+                                                overridden_label)  :
+                                            Proto.RelationType_SetRelates_Req(;
+                                                label = role_label)


Minor formatting issue. Only a single indent from prior line is needed.

Hope, I changed it right yet.

tk3369 · 2021-08-15T17:03:57Z

src/common/rpc/TypeDBStub.jl

@@ -22,5 +22,5 @@ function Cluster_TypeDBStub(channel::gRPCClient.gRPCChannel)
 end

 function ensure_connected(stub::T) where {T<:AbstractTypeDBStub}
-    throw(TypeDBClientException(GENERAL_UNKOWN_ERROR,"function TypeDBStub.jl/ensure_onnected isn't implemented yet"))
+    throw(TypeDBClientException(GENERAL_UNKOWN_ERROR,"function TypeDBStub.jl/ensure_connected isn't implemented yet"))


The stack trace would have shown the file path anyways. I would just change the message string to Not implemented yet

tk3369 · 2021-08-15T17:50:00Z

src/core/CoreSession.jl

-            req = SessionRequestBuilder.close_req(session.sessionID)
-            stub = session.client.core_stub.blockingStub
-            Proto.session_close(stub, gRPCController(), req )
+            if session_alive === true


Suggested change

if session_alive === true

if session_alive

tk3369 · 2021-08-15T17:50:30Z

src/core/CoreTransaction.jl

@@ -58,13 +58,28 @@ end

 function query(transaction::T, request::R, batch::Bool) where {T<:AbstractCoreTransaction, R<:Proto.ProtoType}
    !is_open(transaction) && throw(TypeDBClientException(CLIENT_TRANSACTION_CLOSED))
-    result = single_request(transaction.bidirectional_stream, request, batch)
+    result = []


Not type stable. Same comment for stream function below.

You are right and now it is changed.

tk3369 · 2021-08-15T17:51:42Z

src/core/CoreTransaction.jl

@@ -78,6 +93,7 @@ function close(transaction::T)::Bool where {T<:AbstractCoreTransaction}
        delete!(transaction.session, transaction.transaction_id)
    catch ex
        throw(TypeDBClientException("something went wrong closing Transaction", ex))
+        return false


This line would never be reached since it throws right above...

tk3369 · 2021-08-15T17:54:40Z

test/behaviour/concept/thing/entity/EntitySteps.jl

+@then("entity(person) get instances is empty") do context
+    attr = g.get(context[:cm], EntityType, "person")
+    res_ent = g.get_instances(g.as_remote(attr, context[:transaction]))
+    @expect isempty(res_ent) === true


When the function clearly returns bool, there is no need to compare with true. Same comment for a few other cases below.

Suggested change

@expect isempty(res_ent) === true

@expect isempty(res_ent)

For today it isn't possible to formulate only a variable which contains a Bool in the expect section. A issue is filed and a PR to reach the goal is submitted.

Erik has merged the change what made it possible to fulfill your suggestion.

tk3369 · 2021-08-15T17:56:00Z

test/behaviour/concept/thing/entity/EntitySteps.jl

+    try
+        set_has(context[:transaction], context[:a], context[:bob])
+    catch ex
+        @expect ex !== nothing
+    end


Do you care about the type/cause of exception?

No. Here only the occurence of an exception counts.

for Behavior

codecov · 2021-08-17T10:30:40Z

Codecov Report

Merging #90 (e05d464) into main (9bfb8c8) will increase coverage by 25.30%.
The diff coverage is 17.98%.

@@            Coverage Diff             @@
##            main      #90       +/-   ##
==========================================
+ Coverage   0.00%   25.30%   +25.30%     
==========================================
  Files         10       50       +40     
  Lines       5488     7611     +2123     
==========================================
+ Hits           0     1926     +1926     
- Misses      5488     5685      +197

Impacted Files	Coverage Δ
src/api/TypeDBOptions.jl	`0.00% <0.00%> (ø)`
src/common/exception/ErrorMessage.jl	`0.00% <0.00%> (ø)`
src/common/exception/TypeDBClientException.jl	`0.00% <0.00%> (ø)`
src/common/exception/gRPC_Result_Handling.jl	`0.00% <0.00%> (ø)`
src/common/rpc/TypeDBStub.jl	`0.00% <0.00%> (ø)`
src/concept/ConceptManager.jl	`0.00% <0.00%> (ø)`
src/concept/answer/ConceptMapGroup.jl	`0.00% <0.00%> (ø)`
src/concept/answer/NumericGroup.jl	`0.00% <0.00%> (ø)`
src/concept/thing/Attribute.jl	`0.00% <0.00%> (ø)`
src/concept/type/ThingType.jl	`0.00% <0.00%> (ø)`
... and 89 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 785ed0b...e05d464. Read the comment docs.

documentation purposes.

tk3369

Just a few minor comments. OK to merge now if you wish.

tk3369 · 2021-08-22T06:07:37Z

docs/src/contributing.md

@@ -8,9 +8,9 @@ CurrentModule = TypeDBClient

 So, TypeDB itself provides various ways how to communicate (read from & write to) with the database.


Suggested change

So, TypeDB itself provides various ways how to communicate (read from & write to) with the database.

So, TypeDB itself provides various ways how to communicate (read from & write to) the database.

tk3369 · 2021-08-22T06:15:55Z

src/contribution/contribution.jl

+using ..TypeDBClient
+
+g = TypeDBClient


What do you mean... It seems ok to me? If not, you can at least make g a const.

julia> module Foo foo() = 1 module Bar import ..Foo as g bar() = g.foo() end end Main.Foo julia> Foo.Bar.bar() 1

src/concept/type/RelationType.jl

tk3369 · 2021-08-22T06:19:14Z

Please squash when you merge to main branch. Thanks

suggestions.

= and others added 27 commits June 20, 2021 21:26

Merge branch 'main' into ms/bdd-concept

f4dc069

more concept tests

298d12e

add entity type test

1214181

Merge branch 'ms/bdd-concepts' of github.com:Humans-of-Julia/TypeDBCl…

a29e51d

…ient.jl into ms/bdd-concepts

more tests

ab3709b

Merge remote-tracking branch 'origin/main' into ms/bdd-concepts

e856f19

prepare a test for set_owns

abce568

working on a maybe error in function set_owns

bd3dda4

more bdd tests

844aad5

attribute.feature file tests are fullfilled

53c57db

more tests in Entities

c266bfd

extended Thing.jl with set_has and get_has

d90ad6a

more tests and function get_owners in Attribute.jl

8300201

Entity.feature tests are all implemented and green

5a8b72d

more tests and little tweaks here and there

3bc45bd

more tests and functions in Relation.jl

1bf445e

function delete(Thing) and more tests

a31c6f2

clean up

1b106b4

much more tests

49ceda6

tests in attributetype.feature are fully

9cce40a

implemented

corrected the behavior of transactions for the

a3f2048

case something whent wrong by quering something

more tests and refinements

6621ed9

begin tests in relationtype.feature

bd35b50

implemented relation_type_get_relates_for_role_label and the necessary functions for this.

much more tests and relation.feature fully

9a8c79f

implemented

feature files for graql are deleted and the tests

235bd08

are worked through. The debugging is open.

The tests are done and the feature files for the

2ff23f5

graql part were deleted because they are not relevant for now.

BDD test fully implemented and green :-)

3b5043c

FrankUrbach requested review from tk3369 and ScottPJones August 10, 2021 00:16

changed the runner to run the whole test suite

94de014

properly

This was linked to issues Aug 10, 2021

BDD: Implement "context" code #38

Closed

BDD: Implement common parsing functions #39

Closed

BDD: Implement concept steps #40

Closed

Implement concepts #60

Closed

FrankUrbach added 7 commits August 11, 2021 14:46

implemented function is_deleted for instances of

96e726e

ThingType

Changed all test to fullfill the tasks without

d89b0a1

using TypQL.

fixed a little bug in the RelationSteps.jl

fc1a5ac

remove tags for debugging

56a2332

removed the saved backup files of generated

dad065b

ProtoBuf files

removed a little bug and determine the compat section in

55a28bb

Project.toml

added more precompiled methods to shorten the first

344ac05

startup

tk3369 reviewed Aug 15, 2021

View reviewed changes

FrankUrbach added 5 commits August 16, 2021 03:25

Working through the review of Tom

3fade04

Changed @expect bool now possible without operators

e3759bd

Changed the compat in Project.toml

63e410d

for Behavior

bring unit test to green

7aef0e1

changed a small error to the good.

f36395d

FrankUrbach added 2 commits August 17, 2021 21:48

made some changes to the comments for

9f8f56c

documentation purposes.

Made some actualization in documentation

f0c5619

FrankUrbach requested a review from tk3369 August 19, 2021 00:59

TypeDBClient V1 automation moved this from In progress to Reviewer approved Aug 22, 2021

tk3369 approved these changes Aug 22, 2021

View reviewed changes

FrankUrbach and others added 3 commits August 22, 2021 20:08

added some documentation

82f2543

made small changes according Tom Kwongs

6070c9b

suggestions.

Merge branch 'main' into ms/bdd-concepts

e05d464

FrankUrbach merged commit 8594b55 into main Aug 22, 2021

TypeDBClient V1 automation moved this from Reviewer approved to Done Aug 22, 2021

FrankUrbach deleted the ms/bdd-concepts branch August 22, 2021 18:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ms/bdd concepts #90

Ms/bdd concepts #90

FrankUrbach commented Aug 10, 2021

tk3369 left a comment

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

tk3369 Aug 15, 2021

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

FrankUrbach Aug 16, 2021

tk3369 Aug 15, 2021

FrankUrbach Aug 15, 2021

codecov bot commented Aug 17, 2021 •

edited

Loading

tk3369 left a comment

tk3369 Aug 22, 2021

tk3369 Aug 22, 2021

tk3369 commented Aug 22, 2021

		@@ -8,9 +8,9 @@ CurrentModule = TypeDBClient

		So, TypeDB itself provides various ways how to communicate (read from & write to) with the database.

Ms/bdd concepts #90

Ms/bdd concepts #90

Conversation

FrankUrbach commented Aug 10, 2021

tk3369 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 17, 2021 • edited Loading

Codecov Report

tk3369 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tk3369 commented Aug 22, 2021

codecov bot commented Aug 17, 2021 •

edited

Loading