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

Implement query planner tests #106

Merged
merged 9 commits into from
Jul 22, 2020
Merged

Implement query planner tests #106

merged 9 commits into from
Jul 22, 2020

Conversation

JakeDawkins
Copy link
Contributor

@JakeDawkins JakeDawkins commented Jul 14, 2020

Resolves #103

This PR takes the existing query planner test suite from the apollo-server repo (cradle) and sets up this project to run them whenever there is a runnable query planner.

The signature of the query planner that I've stubbed out here is from this discussion #101 (comment)

Right now, you can run this suite by running cargo test or cargo test -p apollo-query-planner --test cucumber.

Deliverables from #103:

  • Be able to iteratively develop on implementation of a Rust build_query_plan using the .feature files introduced in Update QP tests to use JSON serializer apollo-server#4297.
  • Ensure that the same tests are also executed within CI.
  • All tests to be demonstrated as being executed, but failing, as there is no implementation yet! (And the implementation is out of scope for this issue)
  • Wholesale disabling of these tests in order to allow them to be incrementally disabled as the implementation grows to support their use-cases. This disabling might just be demonstrated as a separate commit.

    this is possible by commenting out the scenarios in the .feature file, but I haven't found a way to skip integration tests yet like you can with regular unit tests using #[ignore]

Copy link
Contributor

@Enrico2 Enrico2 left a comment

Choose a reason for hiding this comment

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

Some stuff

query-planner/tests/cucumber.rs Outdated Show resolved Hide resolved
query-planner/tests/cucumber.rs Outdated Show resolved Hide resolved
query-planner/tests/cucumber.rs Outdated Show resolved Hide resolved
query-planner/tests/cucumber.rs Outdated Show resolved Hide resolved
query-planner/tests/cucumber.rs Outdated Show resolved Hide resolved
query-planner/tests/cucumber.rs Show resolved Hide resolved
});
}

// TODO: how to ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

One of the options is to conditionally compile the entire module:

#[cfg(feature = "with_qp_tests")]
mod cucumber

in mod.rs

(idk if mod.rs is a thing in a tests folder, if not we can put it on mod query_planner_tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we saw yesterday, removing the [[test]] section of the cargo.toml will still compile, but not run the cucumber tests, which seems like a decent middle-ground for now

query-planner/tests/cucumber.rs Show resolved Hide resolved
@JakeDawkins
Copy link
Contributor Author

JakeDawkins commented Jul 21, 2020

I opened cucumber-rs/cucumber#56 to see if there's any way to trim down or remove the full printed output from these tests, since I'd rather not print the full content of the test for every scenario 😞

Update: there is not

@JakeDawkins JakeDawkins force-pushed the jake/test-query-plan branch from 95b2f30 to 4b18001 Compare July 21, 2020 15:58
@JakeDawkins JakeDawkins requested a review from Enrico2 July 21, 2020 16:26
Copy link
Contributor

@Enrico2 Enrico2 left a comment

Choose a reason for hiding this comment

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

Just suggested a comment, but otherwise looks good!

query-planner/Cargo.toml Show resolved Hide resolved
@JakeDawkins JakeDawkins merged commit e8e949a into main Jul 22, 2020
@JakeDawkins JakeDawkins deleted the jake/test-query-plan branch July 22, 2020 16:21
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.

Implement query-planner tests (i.e., Cradle) into this repository
2 participants