martin-g commented on code in PR #571:
URL: https://github.com/apache/avro-rs/pull/571#discussion_r3579039575
##########
avro_derive/src/implementation.rs:
##########
@@ -77,15 +81,46 @@ impl Implementation {
schema_expr: TokenStream,
record_fields_expr: Option<TokenStream>,
field_default_expr: Option<TokenStream>,
+ tests: bool,
) -> Implementation {
- Self {
+ let mut this = Self {
ident,
generics,
schema_expr,
record_fields_expr: record_fields_expr
.unwrap_or_else(|| quote! { ::std::option::Option::None }),
field_default_expr: field_default_expr
.unwrap_or_else(|| quote! { ::std::option::Option::None }),
+ tests: tests.then(Vec::new),
+ };
+
+ this.add_schema_test();
+
+ this
+ }
+
+ pub fn add_test(&mut self, test: Test) {
+ if let Some(tests) = &mut self.tests {
+ tests.push(test);
+ }
+ }
+
+ fn add_schema_test(&mut self) {
+ // Only add the schema test if there are no generics to deal with. I
think it should be possible
+ // to be able to generate the test if there are only lifetimes, but
that needs more experimentation.
+ if self.generics.lifetimes().count() == 0
+ && self.generics.type_params().count() == 0
+ && self.generics.const_params().count() == 0
+ {
+ let ident = &self.ident;
+ self.add_test(Test {
+ name: "schema".to_string(),
Review Comment:
I don't get the idea of this PR :-/
It is invasive by generating tests no one asked for.
The end user will have to opt-out instead of opt-in for those tests.
It is not clear what is being tested too.
1. The `name` is always `"schema"`. Is it really needed ?!
2. The body always prints `"Invalid schema"` to stderr
3. `should_panic` is always `None`
4. The body just calls `get_schema()`, so it tests runtime construction of
the derived Schema and validates its `Name`. But one could assume that the
users make use of the derived schema too in their application and tests, so
this is kinda already covered.
##########
avro/src/serde/derive.rs:
##########
@@ -102,6 +102,11 @@ use crate::{
///
/// Use the schema of the inner field directly. Is only allowed on structs
with only one unskipped field.
///
+/// - `#[avro(test = false)]`
Review Comment:
```suggestion
/// - `#[avro(tests = false)]`
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
