-
Notifications
You must be signed in to change notification settings - Fork 999
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
Subgraph Compositions: Validations #5770
base: krishna/sgc-multiple-sg-sources-fix
Are you sure you want to change the base?
Subgraph Compositions: Validations #5770
Conversation
@@ -37,6 +37,33 @@ const GQL_SCHEMA: &str = r#" | |||
type TestEntity @entity { id: ID! } | |||
"#; | |||
const GQL_SCHEMA_FULLTEXT: &str = include_str!("full-text.graphql"); | |||
// const SOURCE_SUBGRAPH_MANIFEST: &str = include_str!("source.yaml"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor thing - remove the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
b6f7492
to
640e7d1
Compare
ee37a70
to
3944111
Compare
14671db
to
640e7d1
Compare
@@ -1091,54 +1090,6 @@ async fn parse_data_source_context() { | |||
); | |||
} | |||
|
|||
#[tokio::test] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking a second look at this PR, why is this test removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two reasons:
- The test is redudant - Everything this test does is done by the integraiton test
- The test keeps failing because it requires an actual source subgraph.. and some refactoring of the code is required to actually deploy a dummy source subgraph just for the validation to work. But in reality in runner test we mock the triggers and push the triggers directly and no source subgraph is actually needed.
SO i felt its just better to remove this test since, everything this test does is already done by the integration test and the work required to just make the new valdiations work with runner tests is probably unneccessary considering 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for the explanation!
ae35584
to
799a3e9
Compare
06edc2b
to
f38680e
Compare
799a3e9
to
b98760f
Compare
No description provided.