Schema commit time validations #290
Conversation
…ates overrides and inheritance
…and relationtypes tests?
PR Review ChecklistDo not edit the content of this comment. The PR reviewer should simply update this comment by ticking each review item below, as they get completed. Trivial Change
Code
Architecture
|
| | annotation | annotation-category | | ||
| | abstract | abstract | | ||
| | independent | independent | | ||
| # TODO: Only for typeql as we can't parse value types without set value types |
There was a problem hiding this comment.
That's a quite sad limitation for concept api at the moment, but it's fine as we'll have simple typeql tests anyway.
concept/type/owns.feature
Outdated
There was a problem hiding this comment.
This file's tests run for about 3000 seconds (50 minutes) after the reduction of Examples with
bazel test //tests/behaviour/concept/type/... --jobs=1
Maybe it requires separating it into different files, but there is just a huge mass of annotation tests that is around 40-45 minutes, I guess, so it's not totally clear how to do it. At the same time, I don't want to shorten the number of Examples further as some of these Examples really showed some small tricky bugs with specific value types!
There was a problem hiding this comment.
I just updated general type steps to align with the new "language".
concept/validation/migration.feature
Outdated
There was a problem hiding this comment.
I just updated general type steps to align with the new "language".
| @@ -11,490 +11,734 @@ Feature: Schema validation | |||
| Given connection has been opened | |||
| Given connection does not have any database | |||
| Given connection create database: typedb | |||
| Given connection open schema session for database: typedb | |||
| Given session opens transaction of type: write | |||
| Given connection open schema transaction for database: typedb | |||
|
|
|||
|
|
|||
| Scenario: Cyclic type hierarchies are disallowed | |||
There was a problem hiding this comment.
This test can be split into 3 (even 4 with role types) tests and be put into concept files.
|
|
||
| When connection open schema transaction for database: typedb | ||
| When entity(ent1) get plays(rel1:role1) set override: rel0:role0 | ||
| Then transaction commits; fails | ||
|
|
||
|
|
||
| Scenario: A thing-type may not be moved in a way that its plays declarations are hidden by an override |
| Given create attribute type: attr00, with value type: string | ||
| Given attribute(attr00) set abstract: true | ||
| Given create attribute type: attr10, with value type: string | ||
| Scenario: A concrete type must override any ownerships of abstract attributes it inherits |
| When connection open schema transaction for database: typedb | ||
| When entity(ent1) set supertype: ent01; fails | ||
|
|
||
| # TODO: Refactor to functions |
There was a problem hiding this comment.
Gonna rename this file to functions after I move everything else and leave it until we refactor these steps.
| # Then transaction commits; fails | ||
|
|
||
|
|
||
| Scenario: Annotations on ownership overrides must be at least as strict as the overridden ownerships |
| When transaction closes | ||
| When connection open schema transaction for database: typedb | ||
| When entity(ent0n) get owns(attr0) set annotation: @key | ||
| Then transaction commits; fails | ||
|
|
||
|
|
||
| Scenario: Annotations on ownership redeclarations must be stricter than the previous declaration or will be flagged as redundant on commit. |
|
Server PR for these tests: typedb/typedb#7104 |
There was a problem hiding this comment.
As we discussed with @flyingsilverfin , I've renamed this file to function.featureand moved all the possible tests from it to different type/ files.
concept/validation/function.feature
Outdated
There was a problem hiding this comment.
It previously was schema-validation.feature
There was a problem hiding this comment.
I split one owns.feature file into two files. It is still not perfect and can be split further if we just want to speed up the test executions because of the cucumber crate bug (cucumber-rs/cucumber#331), but it should be enough for now to move forward.
There is a specific todo with a link to the cucumber issue in the file's header.
Usage and product changes
We fix old
concept/schema-validationbehaviour tests, reformatting them to the newConceptAPIsteps declarations and changing small details in its logic to follow the new validation rules of3.0.We also fix a number of new
concept/typetests that expectedtransaction commits; fails;results based on the final versions of commit time validations of the3.0server. It is expected that every commit-time validation is now covered by BDD tests.Additionally, this PR presents new steps for
@values,@ranges,@cardannotations (with a separate check ofget cardinalityconstraint), and other small things that were forgotten in the past.Implementation
I'd like to move
schema-validationtests to differenttypefiles as they have just the same meaning and it's hard to split operation time validations with commit time validations (schema validations in migration). Right now all the old tests are still in the old file for easier review.For more details, check out the comments in the changed files.