Skip to content
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

feat: default allowed_gas #4017

Closed
1 task done
milancermak opened this issue Sep 6, 2023 · 5 comments
Closed
1 task done

feat: default allowed_gas #4017

milancermak opened this issue Sep 6, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@milancermak
Copy link
Contributor

Feature Request

Add an attribute that sets the available_gas for every #[test] in a module.

Describe Preferred Solution

Introduce a new module attribute default_available_gas that would set the available gas for each test case in the decorated module. If a test case has its specific available_gas attribute, the test runner would use this specific value to set the gas.

#[cfg(test)]
#[default_available_gas(1000000000)]
mod tests {
    #[test]
    fn test_1() {
        // uses the mod default available gas of 1000000000
    }

    #[test]
    #[available_gas(1000)]
    fn test_perf() {
        // uses the test specific available gas of 1000
    }
}

Describe Alternatives

An alternative would be for the test runner to accept a --available-gas cmdline argument that would act in the same way as the attribute described above. It could also work in tandem where the cmdline arg would be a less specific value. The available gas resolution would then be from most to least specific - test case attribute, module attribute, command line arg, global default.

Related Code

Additional Context

Pretty much everyone just amends their test cases with available_gas and a random big number to have their test cases running. Examples from Kakarot, Renegade finance or this repo. It's annoying and having a way how to globally specify the number would result in a much better devex.

If the feature request is approved, would you be willing to submit a PR?
(Help can be provided if you need assistance submitting a PR)

  • Yes

I'd need some guidance.

@milancermak milancermak added the enhancement New feature or request label Sep 6, 2023
@orizi
Copy link
Collaborator

orizi commented Sep 6, 2023

I'm not sure this is correct - do note that we have functions without loops (i.e. known amount of taken gas).
Should user be unaware for their tests that they take gas?|
In any case - this should either be "infinity" (something of the order of magnitude of u64::max) or nothing at all.
No sense setting it up for all.

@milancermak
Copy link
Contributor Author

Should user be unaware for their tests that they take gas?

IMO the most common use case is "I just want to ensure my tests run, I want to test my code." 95% of the time, these tests are not performance tests, but unit or integration tests. So I'd say it's fine if users (developers) are not aware that tests take gas. In which case, your proposed solution of infinity available gas is great.

For the remaining cases where we want to test the efficiency of a function, the current available_gas attribute works fine.

@orizi
Copy link
Collaborator

orizi commented Sep 7, 2023

so we also probably need to add some option for "const_gas" - to enable ensuring a function does not take gas.
suggestions for such a name?

@milancermak
Copy link
Contributor Author

Honestly, I suck at naming things 😬

@milancermak
Copy link
Contributor Author

Resolved by #4172 ♥️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants