Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Feature public sentryevent.tags #159

Merged
merged 1 commit into from
Sep 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions src/tests/SharpRaven.UnitTests/Data/SentryEventTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#region License

// Copyright (c) 2014 The Sentry Team and individual contributors.
// All rights reserved.
//
// Redistribution and use in source and binary forms, with or without modification, are permitted
// provided that the following conditions are met:
//
// 1. Redistributions of source code must retain the above copyright notice, this list of
// conditions and the following disclaimer.
//
// 2. Redistributions in binary form must reproduce the above copyright notice, this list of
// conditions and the following disclaimer in the documentation and/or other materials
// provided with the distribution.
//
// 3. Neither the name of the Sentry nor the names of its contributors may be used to
// endorse or promote products derived from this software without specific prior written
// permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR
// IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
// FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
// CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
// ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#endregion

using System;

using NUnit.Framework;

using SharpRaven.Data;

namespace SharpRaven.UnitTests.Data
{
[TestFixture]
public class SentryEventTests
{
[Test]
public void ShouldWhenConstructorTagsNotNull()
{
Assert.That(new SentryEvent(new Exception("foo")).Tags, Is.Not.Null);
}

[Test]
public void ShouldNotNullDictionaryWhenAssignNullForTags()
{
var sentryEvent = new SentryEvent(new Exception("foo")) { Tags = null};

Assert.That(sentryEvent.Tags, Is.Not.Null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding this. The test case is great, but the "feature" feels a bit weird. I would really prefer if the setter of Tags threw an ArgumentNullException or similar when trying to set it to null. Setting something to null and then making sure it wasn't set to null doesn't really make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

@genne: Good point. Let's do ArgumentNullException, then. PR incoming? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that this api should work similar to the log4net, i mean when any is wrong with the API, this not interfere of the function of the software.

Copy link
Contributor

Choose a reason for hiding this comment

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

@samukce While I understand that design principle from a logging framework, there's already quite a few throw statements in there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asbjornu but do you agree that some simple exceptions could be avoided?

Copy link
Contributor

@asbjornu asbjornu Sep 20, 2016

Choose a reason for hiding this comment

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

Yes, but in this case, I'm basically ¯\_(ツ)_/¯.

}

[Test]
public void ShouldTheSameDictionaryFromConstructor()
{
var sentryEvent = new SentryEvent(new Exception("foo"));
var beforeDictionary = sentryEvent.Tags;

Assert.That(beforeDictionary, Is.SameAs(sentryEvent.Tags));
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite see the value of this assertion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The get/set execute some procedures how instance new dictionary. Some refactor could inject a bug, for instance, a new dictionary, what could losing data.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point!

}

[Test]
public void ShouldStartNewDictionaryWhenAsignNullForTags()
{
var sentryEvent = new SentryEvent(new Exception("foo"));
var beforeDictionary = sentryEvent.Tags;

sentryEvent.Tags = null;

Assert.That(beforeDictionary, Is.Not.SameAs(sentryEvent.Tags));
}

}
}
1 change: 1 addition & 0 deletions src/tests/SharpRaven.UnitTests/SharpRaven.UnitTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
<Compile Include="Data\HttpRequestBodyConverterTests.cs" />
<Compile Include="Data\JsonPacketFactoryTests.cs" />
<Compile Include="Data\BreadcrumbsRecordTests.cs" />
<Compile Include="Data\SentryEventTests.cs" />
<Compile Include="Data\JsonPacketTests.cs" />
<Compile Include="Data\SentryExceptionTests.cs" />
<Compile Include="Data\SentryMessageTests.cs" />
Expand Down