Skip to content

Make ignoring events test more reliable#1535

Merged
st0012 merged 1 commit intogetsentry:masterfrom
morissetcl:make-ignoring-events-test-more-reliable
Aug 16, 2021
Merged

Make ignoring events test more reliable#1535
st0012 merged 1 commit intogetsentry:masterfrom
morissetcl:make-ignoring-events-test-more-reliable

Conversation

@morissetcl
Copy link
Copy Markdown
Contributor

@morissetcl morissetcl commented Aug 15, 2021

While testing this #1531 I figured out that a test was not really reliable.

If you run:

    it "ignores events that doesn't have a started timestamp" do
      expect do
        ActiveSupport::Notifications.publish "foo", Time.now
      end.not_to raise_error

      expect(transport.events).to be_empty
    end

It will pass.
So I use breadcrumb_buffer to check if we record or not an event.

You can have additional information here #1531 (comment)

Thanks.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 15, 2021 •
edited
Loading

Codecov Report

Merging #1535 (b5e3673) into master (d101202) will increase coverage by 0.55%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1535      +/-   ##
==========================================
+ Coverage   98.21%   98.76%   +0.55%     
==========================================
  Files         218      123      -95     
  Lines       10624     6789    -3835     
==========================================
- Hits        10434     6705    -3729     
+ Misses        190       84     -106
Impacted Files Coverage Δ
...ec/sentry/rails/active_support_breadcrumbs_spec.rb 100.00% <100.00%> (ø)
sentry-raven/lib/raven/backtrace.rb
sentry-raven/lib/raven/processor/cookies.rb
sentry-raven/lib/raven/logger.rb
sentry-raven/lib/raven/interfaces/stack_trace.rb
sentry-raven/lib/raven/transports/dummy.rb
...-raven/spec/raven/integrations/rails/event_spec.rb
sentry-raven/lib/raven/interfaces/exception.rb
sentry-raven/lib/raven/linecache.rb
sentry-raven/lib/raven/interfaces/http.rb
... and 86 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d101202...b5e3673. Read the comment docs.

@st0012
Copy link
Copy Markdown
Contributor

st0012 commented Aug 15, 2021

@morissetcl thanks for the fix 👍
but we don't include non-SDK change into the changelog. can you remove the commit?

@morissetcl
Copy link
Copy Markdown
Contributor Author

morissetcl commented Aug 15, 2021

but we don't include non-SDK change into the changelog. can you remove the commit?

Make sense, sorry first time contribution :)
I am going to remove it.

@morissetcl morissetcl force-pushed the make-ignoring-events-test-more-reliable branch from b5e3673 to ccab270 Compare August 15, 2021 17:30
st0012
st0012 approved these changes Aug 16, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!

@st0012 st0012 merged commit ed0b24b into getsentry:master Aug 16, 2021
@st0012 st0012 mentioned this pull request Aug 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@st0012 st0012 st0012 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

sentry-rails testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@morissetcl @codecov-commenter @st0012