Sovereign-Labs / sovereign-sdk

A framework for building seamlessly scalable and interoperable rollups that can run on any blockchain
https://sovereign.xyz
Apache License 2.0
353 stars 101 forks source link

Add getter generation macro for test purposes #407

Open theochap opened 1 year ago

theochap commented 1 year ago

Description

When testing data structures, we often need to generate public getters to get (normally) private elements of data structures. This issue offers to add a macro that would automatically generate getters for test configuration for the data structures where it is applied.

Example

For the data structure SlotCommit:

pub struct SlotCommit<S: SlotData, B, T> {
    slot_data: S,
    batch_receipts: Vec<BatchReceipt<B, T>>,
    num_txs: usize,
    num_events: usize,
}

The macro would generate


#[cfg(test)]
impl<S: SlotData, B, T> SlotCommit<S, B, T> {
    pub fn slot_data(&self) -> &S {
        &self.slot_data
    }

    pub fn batch_receipts(&self) -> &Vec<BatchReceipt<B, T>> {
        &self.batch_receipts
    }

    pub fn num_txs(&self) -> &usize {
        &self.num_txs
    }

    pub fn num_events(&self) -> &usize {
        &self.num_events
    }
}
theochap commented 1 year ago

I can take this issue if people think that's useful enough!

citizen-stig commented 1 year ago

I think it can be a good idea. @theochap , could you give one or several concrete examples where it would be useful? I am asking, because this solution might bring some burden on test maintenance, because changin private methods can potentially break the tests. Usually it is a good question to ask, why test require access to private fields? If it is black-box testing, then it should be possible to check correctness using public methods. Maybe design of public methods require adjustments?

citizen-stig commented 1 year ago

Also, this only work inside crate, because even in test, other crates are not compiled for testing. Then it means that pub(crate) might be useful to have

neysofu commented 2 months ago

I think practical uses for such macro would be limited and there's several 3rd-party crates that do this already; we could use any of those if this ever becomes necessary. I propose closing this issue.