Skip to content

501: feat: Use macrotest to test the expanded macro code #62

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

Open
martin-augment wants to merge 6 commits into
base: main
Choose a base branch
from
Open

501: feat: Use macrotest to test the expanded macro code #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bd4bf03
feat: Use `macrotest` to test the expanded macro code
Mar 5, 2026
bdac65d
fix: Expanded files are missing license header
Mar 5, 2026
1d7572d
fix: Steps don't share anything
Mar 5, 2026
5dd3e98
fix: Use `taiki-e/install-action` to install `cargo-rdme` and `cargo-…
Mar 5, 2026
01cb0af
fix: Typo in documentation
Kriskras99 Mar 5, 2026
b0f1f60
fix: Only install `cargo-rdme` on `stable-x86_64` and `cargo-expand` …
Mar 5, 2026
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
22 changes: 11 additions & 11 deletions .github/workflows/test-lang-rust-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ jobs:
target: aarch64-unknown-linux-gnu

steps:
- uses: taiki-e/install-action@v2
if: matrix.rust == 'nightly'
with:
tool: cargo-expand
Copy link
Copy Markdown

@augmentcode augmentcode bot Mar 5, 2026

Choose a reason for hiding this comment

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

macrotest comparisons are sensitive to cargo expand output; installing an unpinned cargo-expand version can make nightly CI fail when cargo-expand changes its formatting even if your code hasn’t. Consider pinning a specific cargo-expand version (as macrotest recommends) to reduce test flakiness.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Mar 5, 2026

Choose a reason for hiding this comment

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

value:good-to-have; category:bug; feedback: The Augment AI reviewer is correct! Both macrotest and cargo-expand recommend using a pinned version to prevent random build failures when a new version is released. Prevents test failures when unrelated code changes are made.


- uses: taiki-e/install-action@v2
if: matrix.rust == 'stable' && matrix.runner.target == 'x86_64-unknown-linux-gnu'
with:
tool: cargo-rdme

- name: Checkout
uses: actions/checkout@v6

Expand All @@ -86,21 +96,11 @@ jobs:
components: rustfmt
targets: ${{ matrix.runner.target }}

- name: Cache cargo-rdme
if: matrix.rust == 'stable' && matrix.runner.target == 'x86_64-unknown-linux-gnu'
uses: actions/cache@v5
with:
path: ~/.cargo-${{ matrix.rust }}/cargo-rdme
key: cargo-rdme-

# Check if the doc cumment in avro/src/lib.rs and avro/README.md are in sync.
- name: Run cargo-rdme
# The result is environment independent so one test pattern is enough.
if: matrix.rust == 'stable' && matrix.runner.target == 'x86_64-unknown-linux-gnu'
run: |
cargo install --root ~/.cargo-${{ matrix.rust }}/cargo-rdme --locked cargo-rdme
export PATH=$PATH:~/.cargo-${{ matrix.rust }}/cargo-rdme/bin
cargo rdme --check
run: cargo rdme --check

- name: Rust Format
if: matrix.runner.target != 'wasm32-unknown-unknown'
Expand Down
24 changes: 24 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions avro_derive/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ uuid = { workspace = true }

[dev-dependencies]
apache-avro = { default-features = false, path = "../avro", features = ["derive"] }
macrotest = { version = "1.2.1", default-features = false }
pretty_assertions = { workspace = true }
proptest = { default-features = false, version = "1.10.0", features = ["std"] }
rustversion = "1.0.22"
Expand Down
7 changes: 3 additions & 4 deletions avro_derive/src/enums/plain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,14 @@ pub fn schema_def(
};
symbols.push(name);
}
let full_schema_name = &container_attrs.name;
Ok(quote! {
::apache_avro::schema::Schema::Enum(apache_avro::schema::EnumSchema {
name: ::apache_avro::schema::Name::new(#full_schema_name).expect(&format!("Unable to parse enum name for schema {}", #full_schema_name)[..]),
::apache_avro::schema::Schema::Enum(::apache_avro::schema::EnumSchema {
name,
Copy link
Copy Markdown

@cursor cursor bot Mar 5, 2026

Choose a reason for hiding this comment

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

Enum schema name now differs from record schema name behavior

Medium Severity

The EnumSchema name field now uses the name variable from handle_named_schemas, which is created via Name::new_with_enclosing_namespace (inherits enclosing namespace). Previously it used Name::new(full_schema_name) (ignores enclosing namespace), matching the RecordSchema behavior which still creates its own name via Name::new on line 255 of lib.rs. For enums without an explicit namespace nested inside a namespaced type, the enum's schema name will now include the parent's namespace, while records in the same scenario will not.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Mar 5, 2026

Choose a reason for hiding this comment

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

value:annoying; category:bug; feedback: The Bugbot AI reviewer is not correct! The new way of constructing the Name is similar to the old one - it still passes the full_schema_name, so it won't use any enclosing namespace if the provided name has a namespace.

aliases: #enum_aliases,
doc: #doc,
symbols: vec![#(#symbols.to_owned()),*],
default: #default,
attributes: Default::default(),
attributes: ::std::collections::BTreeMap::new(),
})
})
} else {
Expand Down
Loading