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

chore: fix test and add vscode workspace config #3418

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

npalm
Copy link
Member

@npalm npalm commented Aug 9, 2023

@npalm npalm force-pushed the npalm/fix/unit-test-scale-down branch from 3d28fb2 to 3021367 Compare August 9, 2023 07:56
@npalm npalm enabled auto-merge (rebase) August 9, 2023 07:58
@npalm npalm disabled auto-merge August 9, 2023 07:59
@npalm npalm force-pushed the npalm/fix/unit-test-scale-down branch from 3021367 to 26e3565 Compare August 9, 2023 08:07
@npalm npalm requested a review from JeroenKnoops August 9, 2023 08:28
Copy link
Member

@JeroenKnoops JeroenKnoops left a comment

Choose a reason for hiding this comment

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

LGTM

@npalm npalm merged commit a5f58ae into main Aug 9, 2023
@npalm npalm deleted the npalm/fix/unit-test-scale-down branch August 9, 2023 08:54
@@ -398,6 +396,7 @@ describe('scaleDown', () => {
idleCount: 3,
cron: '* * * * * *',
timeZone: 'Europe/Amsterdam',
evictionStrategy: 'oldest_first',
Copy link
Contributor

Choose a reason for hiding this comment

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

I was specifically trying not to set this because I wanted to ensure that providing no value still defaulted to the right value. Would I make sense to make the SCALE_DOWN_CONFIG as injectable at the method level to make it easier to test? Mutating process.env in tests is just asking for trouble.

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.

3 participants