Skip to content

Commit 387bd81

Browse files
BromeonBromeon
authored
Merge pull request #942 from godot-rust/bugfix/register-docs-tests
Fix `register-docs` feature not being tested
2 parents 361aec8 + fba9f61 commit 387bd81

22 files changed

+69
-51
lines changed

‎.github/workflows/full-ci.yml

+6-4
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ on:
1313

1414

1515
env:
16-
GDEXT_FEATURES: ''
16+
# Applies to all 'register-docs' features across crates.
17+
CLIPPY_FEATURES: '--features register-docs'
18+
TEST_FEATURES: ''
1719
RETRY: ${{ github.workspace }}/.github/other/retry.sh
1820

1921
# ASan options: https://github.com/google/sanitizers/wiki/AddressSanitizerFlags
@@ -99,7 +101,7 @@ jobs:
99101
# Note: could use `-- --no-deps` to not lint dependencies, however it doesn't really speed up and also skips deps in workspace.
100102
- name: "Check clippy"
101103
run: |
102-
cargo clippy --all-targets $GDEXT_FEATURES -- \
104+
cargo clippy --all-targets $CLIPPY_FEATURES -- \
103105
-D clippy::suspicious \
104106
-D clippy::style \
105107
-D clippy::complexity \
@@ -161,10 +163,10 @@ jobs:
161163
run: cargo +nightly update -Z minimal-versions
162164

163165
- name: "Compile tests"
164-
run: cargo test $GDEXT_FEATURES --no-run ${{ matrix.rust-extra-args }}
166+
run: cargo test $TEST_FEATURES --no-run ${{ matrix.rust-extra-args }}
165167

166168
- name: "Test"
167-
run: cargo test $GDEXT_FEATURES ${{ matrix.rust-extra-args }}
169+
run: cargo test $TEST_FEATURES ${{ matrix.rust-extra-args }}
168170

169171

170172
miri-test:

‎.github/workflows/minimal-ci.yml

+6-5
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ on:
2121

2222

2323
env:
24-
GDEXT_FEATURES: ''
25-
# GDEXT_FEATURES: '--features crate/feature'
24+
# Applies to all 'register-docs' features across crates.
25+
CLIPPY_FEATURES: '--features register-docs'
26+
TEST_FEATURES: ''
2627
# GDEXT_CRATE_ARGS: '-p godot-codegen -p godot-ffi -p godot-core -p godot-macros -p godot'
2728
RETRY: ${{ github.workspace }}/.github/other/retry.sh
2829

@@ -96,7 +97,7 @@ jobs:
9697

9798
- name: "Check clippy"
9899
run: |
99-
cargo clippy --all-targets $GDEXT_FEATURES -- \
100+
cargo clippy --all-targets $CLIPPY_FEATURES -- \
100101
-D clippy::suspicious \
101102
-D clippy::style \
102103
-D clippy::complexity \
@@ -120,10 +121,10 @@ jobs:
120121
uses: ./.github/composite/rust
121122

122123
- name: "Compile tests"
123-
run: cargo test $GDEXT_FEATURES --no-run
124+
run: cargo test $TEST_FEATURES --no-run
124125

125126
- name: "Test"
126-
run: cargo test $GDEXT_FEATURES
127+
run: cargo test $TEST_FEATURES
127128

128129

129130
# For complex matrix workflow, see https://stackoverflow.com/a/65434401

‎godot-core/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ homepage = "https://godot-rust.github.io"
1212

1313
[features]
1414
default = []
15-
docs = []
15+
register-docs = []
1616
codegen-rustfmt = ["godot-ffi/codegen-rustfmt", "godot-codegen/codegen-rustfmt"]
1717
codegen-full = ["godot-codegen/codegen-full"]
1818
codegen-lazy-fptrs = [

‎godot-core/src/docs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ pub fn gather_xml_docs() -> impl Iterator<Item = String> {
8989
PluginItem::Struct {
9090
docs: Some(docs), ..
9191
} => map.entry(class_name).or_default().definition = docs,
92+
9293
_ => (),
9394
}
9495
});

‎godot-core/src/init/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ unsafe fn gdext_on_level_init(level: InitLevel) {
143143
unsafe { ensure_godot_features_compatible() };
144144
}
145145
InitLevel::Editor => {
146-
#[cfg(all(since_api = "4.3", feature = "docs"))]
146+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
147147
// SAFETY: Godot binding is initialized, and this is called from the main thread.
148148
unsafe {
149149
crate::docs::register();

‎godot-core/src/lib.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
pub mod builder;
1616
pub mod builtin;
1717
pub mod classes;
18-
#[cfg(all(since_api = "4.3", feature = "docs"))]
18+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
1919
pub mod docs;
2020
#[doc(hidden)]
2121
pub mod possibly_docs {
22-
#[cfg(all(since_api = "4.3", feature = "docs"))]
22+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
2323
pub use crate::docs::*;
2424
}
2525
pub mod global;
@@ -35,7 +35,7 @@ pub use godot_ffi as sys;
3535
// ----------------------------------------------------------------------------------------------------------------------------------------------
3636
// Validations (see also godot/lib.rs)
3737

38-
#[cfg(all(feature = "docs", before_api = "4.3"))]
38+
#[cfg(all(feature = "register-docs", before_api = "4.3"))]
3939
compile_error!("Generating editor docs for Rust symbols requires at least Godot 4.3.");
4040

4141
// ----------------------------------------------------------------------------------------------------------------------------------------------

‎godot-core/src/registry/class.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
243243
is_editor_plugin,
244244
is_internal,
245245
is_instantiable,
246-
#[cfg(all(since_api = "4.3", feature = "docs"))]
246+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
247247
docs: _,
248248
} => {
249249
c.parent_class_name = Some(base_class_name);
@@ -296,7 +296,7 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
296296
PluginItem::InherentImpl(InherentImpl {
297297
register_methods_constants_fn,
298298
register_rpcs_fn: _,
299-
#[cfg(all(since_api = "4.3", feature = "docs"))]
299+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
300300
docs: _,
301301
}) => {
302302
c.register_methods_constants_fn = Some(register_methods_constants_fn);
@@ -315,7 +315,7 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
315315
user_free_property_list_fn,
316316
user_property_can_revert_fn,
317317
user_property_get_revert_fn,
318-
#[cfg(all(since_api = "4.3", feature = "docs"))]
318+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
319319
virtual_method_docs: _,
320320
} => {
321321
c.user_register_fn = user_register_fn;

‎godot-core/src/registry/plugin.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
66
*/
77

8-
#[cfg(all(since_api = "4.3", feature = "docs"))]
8+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
99
use crate::docs::*;
1010
use crate::init::InitLevel;
1111
use crate::meta::ClassName;
@@ -65,7 +65,7 @@ pub struct InherentImpl {
6565
///
6666
/// This function is called in [`UserClass::__before_ready()`](crate::obj::UserClass::__before_ready) definitions generated by the `#[derive(GodotClass)]` macro.
6767
pub register_rpcs_fn: Option<ErasedRegisterRpcsFn>,
68-
#[cfg(all(since_api = "4.3", feature = "docs"))]
68+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
6969
pub docs: InherentImplDocs,
7070
}
7171

@@ -121,7 +121,7 @@ pub enum PluginItem {
121121

122122
/// Whether the class has a default constructor.
123123
is_instantiable: bool,
124-
#[cfg(all(since_api = "4.3", feature = "docs"))]
124+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
125125
docs: Option<StructDocs>,
126126
},
127127

@@ -130,7 +130,7 @@ pub enum PluginItem {
130130

131131
/// Collected from `#[godot_api] impl I... for MyClass`.
132132
ITraitImpl {
133-
#[cfg(all(since_api = "4.3", feature = "docs"))]
133+
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
134134
/// Virtual method documentation.
135135
virtual_method_docs: &'static str,
136136
/// Callback to user-defined `register_class` function.

‎godot-macros/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ homepage = "https://godot-rust.github.io"
1212

1313
[features]
1414
api-custom = ["godot-bindings/api-custom"]
15-
docs = ["dep:markdown"]
15+
register-docs = ["dep:markdown"]
1616
codegen-full = ["godot/__codegen-full"]
1717

1818
[lib]

‎godot-macros/src/class/data_models/field.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ pub struct Field {
1515
pub var: Option<FieldVar>,
1616
pub export: Option<FieldExport>,
1717
pub is_onready: bool,
18-
#[cfg(feature = "docs")]
18+
#[cfg(feature = "register-docs")]
1919
pub attributes: Vec<venial::Attribute>,
2020
}
2121

@@ -28,7 +28,7 @@ impl Field {
2828
var: None,
2929
export: None,
3030
is_onready: false,
31-
#[cfg(feature = "docs")]
31+
#[cfg(feature = "register-docs")]
3232
attributes: field.attributes.clone(),
3333
}
3434
}

‎godot-macros/src/class/data_models/inherent_impl.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ pub fn transform_inherent_impl(mut impl_block: venial::Impl) -> ParseResult<Toke
7474
let (funcs, signals) = process_godot_fns(&class_name, &mut impl_block)?;
7575
let consts = process_godot_constants(&mut impl_block)?;
7676

77-
#[cfg(all(feature = "docs", since_api = "4.3"))]
77+
#[cfg(all(feature = "register-docs", since_api = "4.3"))]
7878
let docs = crate::docs::make_inherent_impl_docs(&funcs, &consts, &signals);
79-
#[cfg(not(all(feature = "docs", since_api = "4.3")))]
79+
#[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
8080
let docs = quote! {};
8181

8282
let signal_registrations = make_signal_registrations(signals, &class_name_obj);

‎godot-macros/src/class/data_models/interface_trait_impl.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ pub fn transform_trait_impl(original_impl: venial::Impl) -> ParseResult<TokenStr
4242
let mut virtual_method_names = vec![];
4343

4444
let prv = quote! { ::godot::private };
45-
#[cfg(all(feature = "docs", since_api = "4.3"))]
45+
#[cfg(all(feature = "register-docs", since_api = "4.3"))]
4646
let docs = crate::docs::make_virtual_impl_docs(&original_impl.body_items);
47-
#[cfg(not(all(feature = "docs", since_api = "4.3")))]
47+
#[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
4848
let docs = quote! {};
4949
for item in original_impl.body_items.iter() {
5050
let method = if let venial::ImplMember::AssocFunction(f) = item {

‎godot-macros/src/class/derive_godot_class.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,13 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
4646

4747
let is_internal = struct_cfg.is_internal;
4848
let base_ty = &struct_cfg.base_ty;
49-
#[cfg(all(feature = "docs", since_api = "4.3"))]
49+
#[cfg(all(feature = "register-docs", since_api = "4.3"))]
5050
let docs = crate::docs::make_definition_docs(
5151
base_ty.to_string(),
5252
&class.attributes,
5353
&fields.all_fields,
5454
);
55-
#[cfg(not(all(feature = "docs", since_api = "4.3")))]
55+
#[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
5656
let docs = quote! {};
5757
let base_class = quote! { ::godot::classes::#base_ty };
5858
let base_class_name_obj = util::class_name_obj(&base_class);

‎godot-macros/src/docs.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pub fn make_definition_docs(
2121
let base_escaped = xml_escape(base);
2222
let desc_escaped = xml_escape(make_docs_from_attributes(description)?);
2323
let members = members
24-
.into_iter()
24+
.iter()
2525
.filter(|x| x.var.is_some() | x.export.is_some())
2626
.filter_map(member)
2727
.collect::<String>();
@@ -112,7 +112,7 @@ fn siphon_docs_from_attributes(doc: &[Attribute]) -> impl Iterator<Item = String
112112
_ => None,
113113
})
114114
.flat_map(|doc| {
115-
doc.into_iter().map(|x| {
115+
doc.iter().map(|x| {
116116
x.to_string()
117117
.trim_start_matches('r')
118118
.trim_start_matches('#')
@@ -126,7 +126,7 @@ fn siphon_docs_from_attributes(doc: &[Attribute]) -> impl Iterator<Item = String
126126

127127
fn xml_escape(value: String) -> String {
128128
// Most strings have no special characters, so this check helps avoid unnecessary string copying
129-
if !value.contains(&['&', '<', '>', '"', '\'']) {
129+
if !value.contains(['&', '<', '>', '"', '\'']) {
130130
return value;
131131
}
132132

‎godot-macros/src/docs/markdown_converter.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ fn walk_node(node: &md::Node, definitions: &HashMap<&str, &str>) -> Option<Strin
7979

8080
Html(html) => html.value.clone(),
8181

82-
_ => walk_nodes(&node.children()?, definitions, ""),
82+
_ => walk_nodes(node.children()?, definitions, ""),
8383
};
8484

8585
Some(bbcode)

‎godot-macros/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
mod bench;
1414
mod class;
1515
mod derive;
16-
#[cfg(all(feature = "docs", since_api = "4.3"))]
16+
#[cfg(all(feature = "register-docs", since_api = "4.3"))]
1717
mod docs;
1818
mod gdextension;
1919
mod itest;

‎godot/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ codegen-rustfmt = ["godot-core/codegen-rustfmt"]
2424
lazy-function-tables = ["godot-core/codegen-lazy-fptrs"]
2525
serde = ["godot-core/serde"]
2626

27-
register-docs = ["godot-macros/docs", "godot-core/docs"]
27+
register-docs = ["godot-macros/register-docs", "godot-core/register-docs"]
2828

2929
api-custom = ["godot-core/api-custom"]
3030
# [version-sync] [[

‎itest/rust/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ default = ["codegen-full"]
1414
codegen-full = ["godot/__codegen-full"]
1515
codegen-full-experimental = ["codegen-full", "godot/experimental-godot-api"]
1616
experimental-threads = ["godot/experimental-threads"]
17-
register-docs = ["godot/register-docs"] # TODO remove as soon as constant_test.rs checks bitfields with #[constant] proc-macro
17+
register-docs = ["godot/register-docs"]
1818
serde = ["dep:serde", "dep:serde_json", "godot/serde"]
1919

2020
# Do not add features here that are 1:1 forwarded to the `godot` crate, unless they are needed by itest itself.

‎itest/rust/src/register_tests/constant_test.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ impl godot::obj::cap::ImplementsGodotApi for HasOtherConstants {
164164
}
165165
}
166166

167-
// TODO once this is done via proc-macro, remove `register-docs` feature from itest, and update CI workflows.
167+
// TODO once this is done via proc-macro, see if `register-docs` is still used in register_docs_test.rs. Otherwise, remove feature from Cargo.toml.
168168
godot::sys::plugin_add!(
169169
__GODOT_PLUGIN_REGISTRY in ::godot::private;
170170
::godot::private::ClassPlugin {
@@ -174,7 +174,7 @@ godot::sys::plugin_add!(
174174
raw: ::godot::private::callbacks::register_user_methods_constants::<HasOtherConstants>,
175175
},
176176
register_rpcs_fn: None,
177-
#[cfg(all(since_api = "4.3", feature = "register-docs"))]
177+
#[cfg(feature = "register-docs")]
178178
docs: ::godot::docs::InherentImplDocs::default(),
179179
}),
180180
init_level: HasOtherConstants::INIT_LEVEL,

‎itest/rust/src/register_tests/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ mod func_test;
1212
mod gdscript_ffi_test;
1313
mod naming_tests;
1414
mod option_ffi_test;
15+
mod register_docs_test;
1516
#[cfg(feature = "codegen-full")]
1617
mod rpc_test;
1718
mod var_test;

‎godot/tests/docs.rs renamed to ‎itest/rust/src/register_tests/register_docs_test.rs

+24-11
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1-
#![cfg(feature = "register-docs")]
21
/*
32
* Copyright (c) godot-rust; Bromeon and contributors.
43
* This Source Code Form is subject to the terms of the Mozilla Public
54
* License, v. 2.0. If a copy of the MPL was not distributed with this
65
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
76
*/
7+
8+
#![cfg(feature = "register-docs")]
9+
10+
use crate::framework::itest;
811
use godot::prelude::*;
912

1013
/// *documented* ~ **documented** ~ [AABB] < [pr](https://github.com/godot-rust/gdext/pull/748)
@@ -187,15 +190,25 @@ impl FairlyDocumented {
187190
fn documented_signal(p: Vector3, w: f64, node: Gd<Node>);
188191
}
189192

190-
#[test]
191-
fn correct() {
193+
#[itest]
194+
fn test_register_docs() {
195+
let xml = find_class_docs("FairlyDocumented");
196+
192197
// Uncomment if implementation changes and expected output file should be rewritten.
193-
// std::fs::write(
194-
// "tests/test_data/docs.xml",
195-
// godot_core::docs::gather_xml_docs().next().unwrap(),
196-
// );
197-
assert_eq!(
198-
include_str!("test_data/docs.xml"),
199-
godot_core::docs::gather_xml_docs().next().unwrap()
200-
);
198+
// std::fs::write("../rust/src/register_tests/res/registered_docs.xml", &xml)
199+
// .expect("failed to write docs XML file");
200+
201+
assert_eq!(include_str!("res/registered_docs.xml"), xml);
202+
}
203+
204+
fn find_class_docs(class_name: &str) -> String {
205+
let mut count = 0;
206+
for xml in godot::docs::gather_xml_docs() {
207+
count += 1;
208+
if xml.contains(class_name) {
209+
return xml;
210+
}
211+
}
212+
213+
panic!("Registered docs for class {class_name} not found in {count} XML files");
201214
}

‎godot/tests/test_data/docs.xml renamed to ‎itest/rust/src/register_tests/res/registered_docs.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ these</description>
2525
</description>
2626
</method>
2727

28-
<method name="virtual_documented">
28+
<method name="_virtual_documented">
2929
<return type="()" />
3030
<param index="0" name="node" type="Gd &lt; Node &gt;" />
3131
<description>

0 commit comments

Comments
 (0)