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.

Sign up for GitHub

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

Jump to bottom

Render models with the new datamodel renderer #3333

Merged
merged 6 commits into from Oct 28, 2022
Merged

Render models with the new datamodel renderer #3333

merged 6 commits into from Oct 28, 2022

Conversation

pimeys
Copy link
Contributor

@pimeys pimeys commented Oct 25, 2022 •
edited

Part of: prisma/prisma#15800

Some of the rendering tests we should maybe redo in introspection. Let's see about that in the upcoming PRs. Lot of the code from here we can remove when we remove DML from introspection.

@pimeys pimeys added this to the 4.6.0 milestone Oct 25, 2022
@pimeys pimeys force-pushed the introspection-engine/use-datamodel-renderer-for-models branch 2 times, most recently from 3153c83 to 6f7af32 Compare October 27, 2022 17:09
@pimeys pimeys marked this pull request as ready for review October 27, 2022 17:09
@pimeys pimeys requested a review from a team as a code owner October 27, 2022 17:09
@pimeys pimeys force-pushed the introspection-engine/use-datamodel-renderer-for-models branch 2 times, most recently from 67901e0 to 381754d Compare October 27, 2022 18:33
@pimeys pimeys force-pushed the introspection-engine/use-datamodel-renderer-for-models branch from 381754d to 235b53a Compare October 27, 2022 19:00
pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/attributes/index_clustering.rs
@@ -120,248 +120,3 @@ fn non_clustered_compound_id_works_on_sql_server() {
clustered: Some(false),
});
}

#[test]
fn do_not_render_id_default_clustering() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are all tested in intro.

pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/attributes/index_positive.rs
@@ -163,22 +162,6 @@ fn multiple_index_must_work() {
});
}

#[test]
fn index_attributes_must_serialize_to_valid_dml() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We test these extensively in intro.

pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/attributes/relations/relations_positive.rs
@@ -249,222 +248,6 @@ fn allow_explicit_fk_name_definition() {
.assert_relation_fk_name(Some("CustomFKName".to_string()));
}

#[test]
fn allow_implicit_fk_name_definition() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The DML has a wiped constraint name for default values.

We test these in the ME.

pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/attributes/unique_positive.rs
@@ -281,22 +280,6 @@ fn single_field_unique_indexes_on_enum_fields_must_work() {
});
}

#[test]
fn unique_attributes_must_serialize_to_valid_dml() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested in intro.

pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/base/unsupported_type.rs
use crate::common::*;

#[test]
fn parse_unsupported_types() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parsing tested in many places in PSL. Rendering tested in intro.

pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/renderer/extended_indexes.rs
@@ -1,116 +0,0 @@
use crate::{common::*, with_header, Provider};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All tested in intro/migrations.

pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/renderer/literals.rs
use psl::dml::{DefaultValue, PrismaValue};

#[test]
fn strings_with_quotes_render_as_escaped_literals() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added one test for this to intro. Migrations are already testing these.

pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/renderer/simplification.rs
use crate::common::*;

#[test]
fn test_must_not_render_relation_fields_with_many_to_many() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Formatter tests?

pimeys
pimeys commented Oct 27, 2022
View reviewed changes
psl/psl/tests/renderer/simplification.rs
}

#[test]
fn test_exclude_default_relation_names_from_rendering() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Formatter?

tomhoule
tomhoule approved these changes Oct 28, 2022
View reviewed changes
Copy link
Contributor

@tomhoule tomhoule left a comment

Choose a reason for hiding this comment

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

Reviewed in a zoom call with @eviefp and @pimeys

:chefkiss:

@pimeys pimeys merged commit 7459cd5 into main Oct 28, 2022
@pimeys pimeys deleted the introspection-engine/use-datamodel-renderer-for-models branch October 28, 2022 08:30
tomhoule added a commit that referenced this pull request Nov 1, 2022
@tomhoule
ie: replicate pre-datamodel-renderer single-field unique behaviour
d6332de 
The first unique index defined on the field wins. This became visible in
introspection CI after
#3333 was merged.

Arguably, we should instead introspect `@@unique()`s on the model for
extra single-field uniques, but that would be a change.
@tomhoule tomhoule mentioned this pull request Nov 1, 2022
Merged
tomhoule added a commit that referenced this pull request Nov 1, 2022
@tomhoule
ie: replicate pre-datamodel-renderer single-field unique behaviour
63086b6 
The first unique index defined on the field wins. This became visible in
introspection CI after
#3333 was merged.

Arguably, we should instead introspect `@@unique()`s on the model for
extra single-field uniques, but that would be a change.
@tomhoule tomhoule mentioned this pull request Nov 1, 2022
Merged
tomhoule added a commit that referenced this pull request Nov 1, 2022
@tomhoule
introspection: merge indexes+relations introspection and reintrospection
e1f5da9 
The path to multiSchema introspection we started on begins with a
refactoring aiming at merging introspection and reintrospection into one
step, and removing all DML usage from the introspection engine. This is
a part of it. See the following past PRs for more context:
#3338 and
#3333.
tomhoule added a commit that referenced this pull request Nov 1, 2022
@tomhoule
introspection: merge index+relation introspection and reintrospection
8e29751 
The path to multiSchema introspection we started on begins with a
refactoring aiming at merging introspection and reintrospection into one
step, and removing all DML usage from the introspection engine. This is
a part of it. See the following past PRs for more context:
#3338 and
#3333.
tomhoule added a commit that referenced this pull request Nov 1, 2022
@tomhoule
introspection: merge index+relation introspection and reintrospection
ed3e9e6 
The path to multiSchema introspection we started on begins with a
refactoring aiming at merging introspection and reintrospection into one
step, and removing all DML usage from the introspection engine. This is
a part of it. See the following past PRs for more context:
#3338 and
#3333.
tomhoule added a commit that referenced this pull request Nov 1, 2022
@tomhoule
introspection: merge index+relation introspection and reintrospection
8408b2f 
The path to multiSchema introspection we started on begins with a
refactoring aiming at merging introspection and reintrospection into one
step, and removing all DML usage from the introspection engine. This is
a part of it. See the following past PRs for more context:
#3338 and
#3333.
tomhoule added a commit that referenced this pull request Nov 2, 2022
@tomhoule
introspection: merge index+relation introspection and reintrospection
6dee8ca 
The path to multiSchema introspection we started on begins with a
refactoring aiming at merging introspection and reintrospection into one
step, and removing all DML usage from the introspection engine. This is
a part of it. See the following past PRs for more context:
#3338 and
#3333.
tomhoule added a commit that referenced this pull request Nov 2, 2022
@tomhoule
introspection: merge index+relation introspection and reintrospection
b543507 
The path to multiSchema introspection we started on begins with a
refactoring aiming at merging introspection and reintrospection into one
step, and removing all DML usage from the introspection engine. This is
a part of it. See the following past PRs for more context:
#3338 and
#3333.
tomhoule added a commit that referenced this pull request Nov 2, 2022
@tomhoule
introspection: merge index+relation introspection and reintrospection
bffd935 
The path to multiSchema introspection we started on begins with a
refactoring aiming at merging introspection and reintrospection into one
step, and removing all DML usage from the introspection engine. This is
a part of it. See the following past PRs for more context:
#3338 and
#3333.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomhoule tomhoule tomhoule approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.6.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pimeys @tomhoule