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

Improved Tests for Schema Size Limit (Issue #312)(Adding Test max encoded schema limit exceeded to pallet/schema) #435

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

HarishKarthickS
Copy link

Hi everyone,

This pull request addresses issue #312 and aims to improve the testing of the maximum schema size limit. These improvements ensure that the system correctly handles scenarios where the schema size might be exceeded.

My Changes:

I've added more comprehensive tests to the existing suite:
One test specifically checks the Schema::create function to verify it throws the MaxEncodedSchemaLimitExceeded error when data is too large.
Another test case focuses on an edge case, making sure the limit check works correctly for data sizes very close to the maximum allowed size.
I've used a mock storage system (MockStorage) to isolate the tests from external dependencies, making them more reliable.
I've included comments throughout the code to explain the purpose of each section (arrange, act, assert) for better understanding.
Benefits:

These additional tests will help us catch potential issues related to exceeding the schema size limit before they occur in production, leading to a more robust system.
Developers can be more confident in how the system handles large schemas.
Clear test cases will make it easier to maintain the code in the future.
Next Steps:

I'd appreciate it if you could review the code changes and test them.
If there are other functions that might be affected by the schema size limit, we could consider adding similar test cases for those as well.
Please double-check that the expected error type (MaxEncodedSchemaLimitExceeded) is accurate.
Thank you for your time and consideration. Please merge this pull request when you're satisfied with the changes.

Best,

Harish Karthick S

let mut mock_storage = MockStorage::new();

// Act
let large_schema_data = vec![0u8; <Test as frame_system::Config>::MaxEncodedSchemaLength + 1];
Copy link
Member

Choose a reason for hiding this comment

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

@HarishKarthickS I don't think this can be done, as the input is compiled to be of type BoundedVec and not just Vec, where MaxEncodedSchemaLength + 1 is impossible due to below,
pub type InputSchemaOf<T> = BoundedVec<u8, <T as Config>::MaxEncodedSchemaLength>;
but it is possible for MaxEncodedSchemaLength - 1 to take place with a BoundedVec since it will be below threshold input set limit.

@VedantKhairnar
Copy link

@HarishKarthickS
Are you working on this PR?

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

Successfully merging this pull request may close these issues.

4 participants