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

Add external types exposed through public api check for CI #434

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add external types exposed through public api check for CI #434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d83c0af
add external types exposed through public api check
u9g Aug 6, 2023
0ff6d35
update job name in ci
u9g Aug 6, 2023
b0a7f39
don't use +nightly when we only install one rust ver
u9g Aug 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ jobs:
- python-tests
- wasm-tests
- js-lint
- exported-api
if: ${{ success() || failure() }} # Run this job even if a dependency has failed.
steps:
- name: Job outcomes
Expand All @@ -39,6 +40,7 @@ jobs:
echo "wasm-tests: ${{ needs.wasm-tests.result }}"
echo "js-lint: ${{ needs.js-lint.result }}"
echo "rust-rustdoc-wasm: ${{ needs.rust-rustdoc-wasm.result }}"
echo "exported-api: ${{ needs.exported-api.result }}"

# Fail this required job if any of its dependent jobs have failed.
#
Expand All @@ -59,6 +61,8 @@ jobs:
run: exit 1
- if: ${{ needs.rust-rustdoc-wasm.result != 'success' }}
run: exit 1
- if: ${{ needs.exported-api.result != 'success' }}
run: exit 1

python-tests:
name: Python tests and maturin build
Expand Down Expand Up @@ -172,6 +176,28 @@ jobs:
RUSTDOCFLAGS: -D warnings
run: cargo doc --workspace --all-features --no-deps --document-private-items

exported-api:
name: Check for new exported types
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
with:
persist-credentials: false

- name: Install rust
uses: dtolnay/rust-toolchain@master
with:
toolchain: "nightly-2023-05-31"

- uses: Swatinem/rust-cache@v2

- name: Check Exported API
run: |
cargo install --locked cargo-check-external-types
Copy link
Owner

Choose a reason for hiding this comment

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

This is going to rebuild on every PR update, which is going to be horrifically slow. The tool's doesn't offer prebuilt binaries, and it's not part of cargo-quickinstall's prebuilt list either.

Almost tempted to not use it over this, to be honest.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not super upset over it as it takes approximately the same 2 minutes to run the tests. We can also either ask them to offer prebuilt binaries or just build it ourselves on every commit in a different repo similar to what cargo-quickinstall does.

Is there any way we can ask them to add it to cargo-quickinstall?

cargo check-external-types --all-features --config trustfall/external-types.toml --manifest-path trustfall/Cargo.toml
cargo check-external-types --all-features --config trustfall/external-types.toml --manifest-path trustfall/Cargo.toml

rust-tests:
name: Run tests
runs-on: ubuntu-latest
Expand Down
38 changes: 38 additions & 0 deletions trustfall/external-types.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
allowed_external_types = [
"anyhow::Result",
"trustfall_core::accessor_property",
"trustfall_core::field_property",
"trustfall_core::interpreter::Adapter",
"trustfall_core::interpreter::basic_adapter::BasicAdapter",
"trustfall_core::interpreter::ContextIterator",
"trustfall_core::interpreter::ContextOutcomeIterator",
"trustfall_core::interpreter::DataContext",
"trustfall_core::interpreter::helpers::correctness::check_adapter_invariants",
"trustfall_core::interpreter::helpers::resolve_coercion_using_schema",
"trustfall_core::interpreter::helpers::resolve_coercion_with",
"trustfall_core::interpreter::helpers::resolve_neighbors_with",
"trustfall_core::interpreter::helpers::resolve_property_with",
"trustfall_core::interpreter::helpers::resolve_typename",
"trustfall_core::interpreter::hints::candidates::CandidateValue",
"trustfall_core::interpreter::hints::candidates::Range",
"trustfall_core::interpreter::hints::dynamic::DynamicallyResolvedValue",
"trustfall_core::interpreter::hints::EdgeInfo",
"trustfall_core::interpreter::hints::QueryInfo",
"trustfall_core::interpreter::hints::ResolveEdgeInfo",
"trustfall_core::interpreter::hints::ResolveInfo",
"trustfall_core::interpreter::hints::vertex_info::VertexInfo",
"trustfall_core::interpreter::Typename",
"trustfall_core::interpreter::VertexIterator",
"trustfall_core::interpreter::VertexIterator",
"trustfall_core::ir::EdgeParameters",
"trustfall_core::ir::Eid",
"trustfall_core::ir::value::FieldValue",
"trustfall_core::ir::value::TransparentValue",
"trustfall_core::ir::Vid",
"trustfall_core::schema::adapter::SchemaAdapter",
"trustfall_core::schema::Schema",
"trustfall_core::serialization::TryIntoStruct",
"trustfall_derive::TrustfallEnumVertex",
"trustfall_derive::Typename",
"trustfall::provider::resolve_neighbors_with",
]
48 changes: 48 additions & 0 deletions trustfall_core/external-types.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
allowed_external_types = [
"trustfall_core::accessor_property",
"trustfall_core::field_property",
"trustfall_core::interpreter::Adapter",
"trustfall_core::interpreter::basic_adapter::BasicAdapter",
"trustfall_core::interpreter::ContextIterator",
"trustfall_core::interpreter::ContextOutcomeIterator",
"trustfall_core::interpreter::DataContext",
"trustfall_core::interpreter::helpers::correctness::check_adapter_invariants",
"trustfall_core::interpreter::helpers::resolve_coercion_using_schema",
"trustfall_core::interpreter::helpers::resolve_coercion_with",
"trustfall_core::interpreter::helpers::resolve_neighbors_with",
"trustfall_core::interpreter::helpers::resolve_property_with",
"trustfall_core::interpreter::helpers::resolve_typename",
"trustfall_core::interpreter::hints::candidates::CandidateValue",
"trustfall_core::interpreter::hints::candidates::Range",
"trustfall_core::interpreter::hints::dynamic::DynamicallyResolvedValue",
"trustfall_core::interpreter::hints::EdgeInfo",
"trustfall_core::interpreter::hints::QueryInfo",
"trustfall_core::interpreter::hints::ResolveEdgeInfo",
"trustfall_core::interpreter::hints::ResolveInfo",
"trustfall_core::interpreter::hints::vertex_info::VertexInfo",
"trustfall_core::interpreter::Typename",
"trustfall_core::interpreter::VertexIterator",
"trustfall_core::interpreter::VertexIterator",
"trustfall_core::ir::EdgeParameters",
"trustfall_core::ir::Eid",
"trustfall_core::ir::value::FieldValue",
"trustfall_core::ir::value::TransparentValue",
"trustfall_core::ir::Vid",
"trustfall_core::schema::adapter::SchemaAdapter",
"trustfall_core::schema::Schema",
"trustfall_core::serialization::TryIntoStruct",
"trustfall_derive::TrustfallEnumVertex",
"trustfall_derive::Typename",
"trustfall::provider::resolve_neighbors_with",

"serde::de::*",
"serde::ser::*",

"async_graphql_parser::Error",
"async_graphql_parser::pos::Pos",
"async_graphql_parser::types::executable::ExecutableDocument",
"async_graphql_parser::types::service::ServiceDocument",
"async_graphql_parser::types::Type",
"async_graphql_value::ConstValue",
"async_graphql_value::Value",
]