Skip to content

test(pulse): add tests for price updates removal and max price IDs validation #2676

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

Merged
merged 5 commits into from
May 27, 2025

Conversation

devin-ai-integration[bot]
Copy link
Contributor

Add tests for PulseScheduler

This PR adds two new test cases to the PulseScheduler test suite:

  1. testUpdateSubscriptionRemovesPriceUpdatesForRemovedPriceIds() - Verifies that after updating a subscription's price IDs list, the priceUpdates mapping no longer contains price data for removed price IDs.

  2. testUpdateSubscriptionRevertsWithTooManyPriceIds() - Asserts that updateSubscription reverts if priceIds.length > MAX_PRICE_IDS_PER_SUBSCRIPTION.

Both tests pass when run with forge test and the code passes linting checks.

Link to Devin run: https://app.devin.ai/sessions/01e1cd793ef8465090357079505e07c0
Requested by: Tejas Badadare

Co-Authored-By: Tejas Badadare <tejas@dourolabs.xyz>
Copy link

vercel bot commented May 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api-reference ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 4:45pm
component-library ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 4:45pm
developer-hub ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 4:45pm
entropy-debugger ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 4:45pm
entropy-explorer ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 4:45pm
insights ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 4:45pm
proposals ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 4:45pm
staking ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 4:45pm

Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@tejasbadadare tejasbadadare changed the title feat: add tests for price updates removal and max price IDs validation test(pulse): add tests for price updates removal and max price IDs validation May 12, 2025
Co-Authored-By: Tejas Badadare <tejas@dourolabs.xyz>
Copy link
Contributor Author

Closing due to inactivity for more than 7 days.

Copy link
Collaborator

@ali-behjati ali-behjati left a comment

Choose a reason for hiding this comment

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

Devin please fix my comments.

@tejasbadadare you can approve and merge it when it fixes the tests

);

// - Verify that the removed price ID is not included in the returned prices
for (uint i = 0; i < allPricesAfterUpdate.length; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this for is unnecessary, but why we cannot check all the priceIds directly? Does the order change?

allPricesAfterUpdate.length,
updatedParams.priceIds.length,
"Number of returned prices should match number of price IDs in subscription"
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we also test the behaviour of getting the price of the removed feed?

Co-Authored-By: Tejas Badadare <tejas@dourolabs.xyz>
@vercel vercel bot temporarily deployed to Preview – entropy-debugger May 27, 2025 12:26 Inactive
@vercel vercel bot temporarily deployed to Preview – staking May 27, 2025 12:26 Inactive
@vercel vercel bot temporarily deployed to Preview – entropy-explorer May 27, 2025 12:26 Inactive
@vercel vercel bot temporarily deployed to Preview – insights May 27, 2025 12:26 Inactive
@vercel vercel bot temporarily deployed to Preview – proposals May 27, 2025 12:26 Inactive
@vercel vercel bot temporarily deployed to Preview – component-library May 27, 2025 12:26 Inactive
@vercel vercel bot temporarily deployed to Preview – api-reference May 27, 2025 12:26 Inactive
@tejasbadadare tejasbadadare merged commit 58c3523 into main May 27, 2025
12 checks passed
@tejasbadadare tejasbadadare deleted the devin/1747075585-add-pulse-scheduler-tests branch May 27, 2025 22:10
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.

2 participants