refactor(macros): cleanup macros, add better error handling, dedup code, DX #472
Conversation
…warning, dedup code, improve DX
…de, show nested example
There was a problem hiding this comment.
Looks good, some comments and would appreciate more reviews on this @Serial-ATA @shekohex
| #[test] | ||
| fn test_jobs_invalid_cases() { | ||
| let t = TestCases::new(); | ||
| t.compile_fail("tests/invalid_cases/job/*.rs"); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
The valid test cases you fixed below aren't running now ya? Can we make them run? Or it's simply enough to define them above and ensure they compile?
There was a problem hiding this comment.
Before, they weren't technically running, only checking for syntactical correctness. Now, it does the same thing but implicitly by being attached to the module tree. We could make them run once to check input/output correctness, though, maybe in another PR since it would be beyond the scope of this task.
| pre_processor = pre_process, | ||
| post_processor = post_process, | ||
| ), | ||
| )] | ||
| // Maps a boolean value obtained from pre-processing to a u8 value | ||
| pub async fn web_poller(value: bool, count: Arc<AtomicUsize>) -> Result<u8, Infallible> { | ||
| pub async fn web_poller_test(value: bool, count: Arc<AtomicUsize>) -> Result<u8, Infallible> { |
There was a problem hiding this comment.
Can't we test this in the blueprint examples repo? It's getting to be a bit of a nested mess IMO. PeriodicListener with WebPoller and then another PeriodicListener with a WebPollerTest containing a WebPoller.
I would suggest getting CI to test the example blueprint instead? Would that accomplish your goal here? It would also be good to have a PeriodicListener example as a simple cron-job no inner listener too, no complexity, just "here's how you trigger your job every 2 seconds".
There was a problem hiding this comment.
We currently don't have a clean way of testing blueprints unless we inject test-related code into them (like I did for the web_poller_test). While we could test the one inside examples, we would have to add more code to it, confusing the user by mixing prod code with test code. At least with web_poller_test, I just need to import the already-existing code inside examples, then add a thin wrapper over it where I am then able to inject the test-related code.
There was a problem hiding this comment.
Overall Looks good!
aside from the removed test cases, we need to expand more and add a test case for each scenario (AI Could be useful here for generating different scenarios); Testing here is very important to make sure changes to the macros or other things does not break the SDK.
| /// Tuple | ||
| Tuple(Vec<FieldType>), |
There was a problem hiding this comment.
This needs to be updated and added to the runtime, please open a PR on tangle or a task so we are aware of it.
CC @Serial-ATA to maybe support this into blueprint-serde.
There was a problem hiding this comment.
Addresses #407 and #381