sentry-ruby icon indicating copy to clipboard operation
sentry-ruby copied to clipboard

Published 20 hours ago • verified publisher icon getsentry

fix: Don't mutate original enabled_environments

Open ixti opened this issue 1 year ago • 3 comments

Ensure enabled_environments contains test environment without mutating original list. Otherwise it can cause unexpected spec failures in case original config has unfrozen Array, or fail in attempt to mutate said array.

Alternative approach would be to override Sentry::Configuration#dup to dup underlying arrays and hashes.

See: https://github.com/getsentry/sentry-ruby/pull/2269#issuecomment-2002637738

ixti avatar Mar 17 '24 22:03 ixti

Codecov Report

Merging #2275 (ebf04a4) into master (ffffce9) will decrease coverage by 0.05%. The diff coverage is 100.00%.

:exclamation: Current head ebf04a4 differs from pull request most recent head a573841. Consider uploading reports for the commit a573841 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2275      +/-   ##
==========================================
- Coverage   97.61%   97.56%   -0.05%     
==========================================
  Files         112      112              
  Lines        4154     4155       +1     
==========================================
- Hits         4055     4054       -1     
- Misses         99      101       +2
Components Coverage Δ
sentry-ruby 98.27% <100.00%> (-0.04%) :arrow_down:
sentry-rails 95.05% <ø> (-0.18%) :arrow_down:
sentry-sidekiq 94.70% <ø> (ø)
sentry-resque 90.76% <ø> (ø)
sentry-delayed_job 95.60% <ø> (ø)
sentry-opentelemetry 100.00% <ø> (ø)
Files Coverage Δ
sentry-ruby/lib/sentry/test_helper.rb 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

codecov[bot] avatar Mar 20 '24 10:03 codecov[bot]

Thanks for the PR, but why did you find it necessary to freeze the enabled_environments array in the first place?

st0012 avatar Apr 29 '24 10:04 st0012

My tests were randomly passing randomly failing. Adding a freeze helped to nail down that Sentry helper was changing configuration inplace - thus specs became order-dependent

ixti avatar May 02 '24 20:05 ixti

superseded by #2317

sl0thentr0py avatar Jun 06 '24 14:06 sl0thentr0py

I still think |= better reflects the desire than += :D

ixti avatar Jul 23 '24 16:07 ixti

didn’t work for me :(

Vagab avatar Jul 23 '24 16:07 Vagab

didn’t work for me :(

Oh. That's weird. Either way, as long as Sentry does not mutate original config - we're good :D

:heart: :heart: :heart:

ixti avatar Jul 23 '24 16:07 ixti