-
Notifications
You must be signed in to change notification settings - Fork 248
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 recorded::test
macro for recorded tests
#1926
base: main
Are you sure you want to change the base?
Conversation
Will be more consistent with upcoming azure_core_macros, and may not contain *just* derive macros anyway. Will keep feature as "derive", though, for derive macros.
I also had to bump the MSRV to 1.80 to use the stable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this. My only thought, is maybe we rename this "recorded_test" or just call the attribute "test" and but it in a recorded
module with the intent of being referenced as #[recorded::test]
? Since it's effectively replacing the test marker entirely and the user is expected to NOT apply a standard test attribute, I think that including "test" in the name would make that clear.
I'm fine with bumping our MSRV, though I'll be honest I'm not sure it's necessary to cache the test mode value like this. It should work just fine though, and I like getting new Rust versions 😁 |
Once this repo really starts churning, we'll have thousands of tests and I imagine that time will add up. But we could see. I'd rather merge this as-is and will do a subsequent PR to remove it so we have a history of it and the decisions made. I also want to do a follow-up PR (or more) to refactor how this gets exposed, which leads me into...
I was initially trying to avoid a long name like use azure_core_test::{recorded, TestContext, TestMode};
#[recorded::test]
async fn foo(ctx: TestContext) {
if ctx.test_mode() == TestMode::Live {
todo!();
}
todo!()
} I mean, if someone got in the habit of just writing Maybe we should have a use azure_core_test::prelude::*;
#[recorded::test]
async fn foo(ctx: TestContext) {
if ctx.test_mode() == TestMode::Live {
todo!();
}
todo!()
} @RickWinter @JeffreyRichter what do you think? I honestly do like the look of |
Also makes the attribute `#[recorded::test]` which, IMO, looks a bit better.
recorded
macro for recorded testsrecorded::test
macro for recorded tests
We may need to set env vars for our Windows agents to find OpenSSL, which does appear to be installed. See Azure#1746. For now, I'm removing `--all-features` from this PR since that issue is already tracking adding them separately.
Rename typespec_derive to typespec_macros
Will be more consistent with upcoming azure_core_macros, and may not contain just derive macros anyway. Will keep feature as "derive", though, for derive macros.
Remove 128-bit number functions for Cosmos
Implement
#[recorded]
attribute macroAllow live-only tests with no parameters
Replace test_e2e feature with
recorded
attribute macro