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

Add support for unit testing for user context #271

Closed
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
3 changes: 3 additions & 0 deletions OptimizelySDK.Net35/OptimizelySDK.Net35.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@
<Compile Include="..\OptimizelySDK\IOptimizely.cs">
<Link>IOptimizely.cs</Link>
</Compile>
<Compile Include="..\OptimizelySDK\IOptimizelyUserContext.cs">
<Link>IOptimizelyUserContext.cs</Link>
</Compile>
<Compile Include="..\OptimizelySDK\Logger\DefaultLogger.cs">
<Link>Logger\DefaultLogger.cs</Link>
</Compile>
Expand Down
3 changes: 3 additions & 0 deletions OptimizelySDK.Net40/OptimizelySDK.Net40.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@
</Reference>
</ItemGroup>
<ItemGroup>
<Compile Include="..\OptimizelySDK\IOptimizelyUserContext.cs">
<Link>IOptimizelyUserContext.cs</Link>
</Compile>
<Compile Include="..\OptimizelySDK\AudienceConditions\AndCondition.cs">
<Link>AudienceConditions\AndCondition.cs</Link>
</Compile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@
<Link>AtomicProjectConfigManager.cs</Link>
</Compile>
<Compile Include="..\OptimizelySDK\OptimizelyFactory.cs">
<Link>OptimizelyFactory.cs</Link>
<Link>OptimizelyFactory.cs</Link>
</Compile>
<Compile Include="..\OptimizelySDK\IOptimizelyUserContext.cs">
<Link>IOptimizelyUserContext.cs</Link>
</Compile>
<Compile Include="..\OptimizelySDK\Event\Entity\ConversionEvent.cs">
<Link>ConversionEvent.cs</Link>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
<Compile Include="..\OptimizelySDK\IOptimizely.cs">
<Link>IOptimizely.cs</Link>
</Compile>
<Compile Include="..\OptimizelySDK\IOptimizelyUserContext.cs">
<Link>IOptimizelyUserContext.cs</Link>
</Compile>
<Compile Include="..\OptimizelySDK\Optimizely.cs">
<Link>Optimizely.cs</Link>
</Compile>
Expand Down
1 change: 1 addition & 0 deletions OptimizelySDK.Tests/OptimizelySDK.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
<Compile Include="TestBucketer.cs" />
<Compile Include="BucketerTest.cs" />
<Compile Include="ProjectConfigTest.cs" />
<Compile Include="TestSetup.cs" />
<Compile Include="UtilsTests\ConditionParserTest.cs" />
<Compile Include="UtilsTests\EventTagUtilsTest.cs" />
<Compile Include="UtilsTests\ExceptionExtensionsTest.cs" />
Expand Down
14 changes: 7 additions & 7 deletions OptimizelySDK.Tests/OptimizelyUserContextTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void SetUp()
public void OptimizelyUserContextWithAttributes()
{
var attributes = new UserAttributes() { { "house", "GRYFFINDOR" } };
OptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, attributes, ErrorHandlerMock.Object, LoggerMock.Object);
IOptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, attributes, ErrorHandlerMock.Object, LoggerMock.Object);

Assert.AreEqual(user.GetOptimizely(), Optimizely);
Assert.AreEqual(user.GetUserId(), UserID);
Expand All @@ -73,7 +73,7 @@ public void OptimizelyUserContextWithAttributes()
[Test]
public void OptimizelyUserContextNoAttributes()
{
OptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, null, ErrorHandlerMock.Object, LoggerMock.Object);
IOptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, null, ErrorHandlerMock.Object, LoggerMock.Object);

Assert.AreEqual(user.GetOptimizely(), Optimizely);
Assert.AreEqual(user.GetUserId(), UserID);
Expand All @@ -84,7 +84,7 @@ public void OptimizelyUserContextNoAttributes()
public void SetAttribute()
{
var attributes = new UserAttributes() { { "house", "GRYFFINDOR" } };
OptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, attributes, ErrorHandlerMock.Object, LoggerMock.Object);
IOptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, attributes, ErrorHandlerMock.Object, LoggerMock.Object);

user.SetAttribute("k1", "v1");
user.SetAttribute("k2", true);
Expand All @@ -104,7 +104,7 @@ public void SetAttribute()
[Test]
public void SetAttributeNoAttribute()
{
OptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, null, ErrorHandlerMock.Object, LoggerMock.Object);
IOptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, null, ErrorHandlerMock.Object, LoggerMock.Object);

user.SetAttribute("k1", "v1");
user.SetAttribute("k2", true);
Expand All @@ -120,7 +120,7 @@ public void SetAttributeNoAttribute()
public void SetAttributeOverride()
{
var attributes = new UserAttributes() { { "house", "GRYFFINDOR" } };
OptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, attributes, ErrorHandlerMock.Object, LoggerMock.Object);
IOptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, attributes, ErrorHandlerMock.Object, LoggerMock.Object);

user.SetAttribute("k1", "v1");
user.SetAttribute("house", "v2");
Expand All @@ -134,7 +134,7 @@ public void SetAttributeOverride()
public void SetAttributeNullValue()
{
var attributes = new UserAttributes() { { "k1", null } };
OptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, attributes, ErrorHandlerMock.Object, LoggerMock.Object);
IOptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, attributes, ErrorHandlerMock.Object, LoggerMock.Object);

var newAttributes = user.GetAttributes();
Assert.AreEqual(newAttributes["k1"], null);
Expand All @@ -151,7 +151,7 @@ public void SetAttributeNullValue()
[Test]
public void SetAttributeToOverrideAttribute()
{
OptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, null, ErrorHandlerMock.Object, LoggerMock.Object);
IOptimizelyUserContext user = new OptimizelyUserContext(Optimizely, UserID, null, ErrorHandlerMock.Object, LoggerMock.Object);


Assert.AreEqual(user.GetOptimizely(), Optimizely);
Expand Down
41 changes: 41 additions & 0 deletions OptimizelySDK.Tests/TestSetup.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2021, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

using NUnit.Framework;
using System.Globalization;
using System.Threading;

namespace OptimizelySDK.Tests
{
[SetUpFixture]
public class TestSetup
{
[SetUp]
public void Init()
{
/* There are some issues doing assertions on tests with floating point numbers using the .ToString()
* method, as it's culture dependent. EG: TestGetFeatureVariableValueForTypeGivenFeatureFlagIsNotEnabledForUser,
* assigning the culture to English will make this kind of tests to work on others culture based systems. */
Thread.CurrentThread.CurrentCulture = CultureInfo.CreateSpecificCulture("en-US");
}

[TearDown]
public void Cleanup()
{
// Empty, but required: https://nunit.org/nunitv2/docs/2.6.4/setupFixture.html
}
}
}
2 changes: 1 addition & 1 deletion OptimizelySDK/IOptimizely.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public interface IOptimizely
/// <param name="userId">The user ID to be used for bucketing.</param>
/// <param name="userAttributes">The user's attributes</param>
/// <returns>OptimizelyUserContext | An OptimizelyUserContext associated with this OptimizelyClient.</returns>
OptimizelyUserContext CreateUserContext(string userId, UserAttributes userAttributes = null);
IOptimizelyUserContext CreateUserContext(string userId, UserAttributes userAttributes = null);
Copy link
Contributor

Choose a reason for hiding this comment

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

this will break existing functionality.
e.g. someone used like this.
OptimizelyUserContext ouc = obj.CreateUserContext(...) /// value can't be set when returning IOptimizelyUserContext.
We should return to OptimizelyUserContext.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the idea of the PR is to get the interface, as using the explicit type when declaring is in disuse nowadays. Usually code analyzers mark this as warnings. EG:

Then, is just a matter of replacing:
var ouc = obj.CreateUserContext(...);

I think that, the benefits of being able to write unit tests is by far higher for the majority of developers, than the cost of replacing the explicit type by var.

What to you thnik?.

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, if the coding standards don't allow the usage of var, is just a matter of using the interface instead:

IOptimizelyUserContext ouc = obj.CreateUserContext(...)

Copy link
Contributor

Choose a reason for hiding this comment

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

In fact, if the coding standards don't allow the usage of var, is just a matter of using the interface instead:

IOptimizelyUserContext ouc = obj.CreateUserContext(...)

Unfortunately it's not just a matter of coding standards, this would technically require a breaking change to implement in the current version.

We agree this is a worthwhile enhancement for the next major version of the SDK and will add it to our backlog for when that next release is planned.

What we can do in the interim is provide a virtual that will get the user context provided, which can then be overridden for your testing purposes.

Let us know if you think that would be a good stop gap until we can provide the interface change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed, this is a breaking change, if you guys consider it useful and want to include this on the next major release, is perfect.
Then, we can wait, I prefer to close the PR and wait.
Many thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But well, you are right, making them virtual is just a nice quick win, and that will allow the mocking library, to intercept the calls and set expectations.

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 created the PR with the virtual keyword. It's here: #280

It also includes the fix for unit tests running on non English systems.

I'll then, close this PR.

Many thanks!.


/// <summary>
/// Sends conversion event to Optimizely.
Expand Down
112 changes: 112 additions & 0 deletions OptimizelySDK/IOptimizelyUserContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* Copyright 2020-2021, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

using OptimizelySDK.Entity;
using OptimizelySDK.OptimizelyDecisions;
using System.Collections.Generic;

namespace OptimizelySDK
{
public interface IOptimizelyUserContext
{
/// <summary>
/// Returns a decision result ({@link OptimizelyDecision}) for a given flag key and a user context, which contains all data required to deliver the flag.
/// <ul>
/// <li>If the SDK finds an error, it’ll return a decision with <b>null</b> for <b>variationKey</b>. The decision will include an error message in <b>reasons</b>.
/// </ul>
/// </summary>
/// <param name="key">A flag key for which a decision will be made.</param>
/// <returns>A decision result.</returns>
OptimizelyDecision Decide(string key);

/// <summary>
/// Returns a decision result ({@link OptimizelyDecision}) for a given flag key and a user context, which contains all data required to deliver the flag.
/// <ul>
/// <li>If the SDK finds an error, it’ll return a decision with <b>null</b> for <b>variationKey</b>. The decision will include an error message in <b>reasons</b>.
/// </ul>
/// </summary>
/// <param name="key">A flag key for which a decision will be made.</param>
/// <param name="options">A list of options for decision-making.</param>
/// <returns>A decision result.</returns>
OptimizelyDecision Decide(string key, OptimizelyDecideOption[] options);

/// <summary>
/// Returns a key-map of decision results ({@link OptimizelyDecision}) for all active flag keys.
/// </summary>
/// <returns>A dictionary of all decision results, mapped by flag keys.</returns>
Dictionary<string, OptimizelyDecision> DecideAll();

/// <summary>
/// Returns a key-map of decision results ({@link OptimizelyDecision}) for all active flag keys.
/// </summary>
/// <param name="options">A list of options for decision-making.</param>
/// <returns>All decision results mapped by flag keys.</returns>
Dictionary<string, OptimizelyDecision> DecideAll(OptimizelyDecideOption[] options);

/// <summary>
/// Returns a key-map of decision results for multiple flag keys and a user context.
/// </summary>
/// <param name="keys">list of flag keys for which a decision will be made.</param>
/// <returns>A dictionary of all decision results, mapped by flag keys.</returns>
Dictionary<string, OptimizelyDecision> DecideForKeys(string[] keys);

/// <summary>
/// Returns a key-map of decision results for multiple flag keys and a user context.
/// </summary>
/// <param name="keys">list of flag keys for which a decision will be made.</param>
/// <param name="options">An array of decision options.</param>
/// <returns></returns>
Dictionary<string, OptimizelyDecision> DecideForKeys(string[] keys, OptimizelyDecideOption[] options);

/// <summary>
/// Returns copy of UserAttributes associated with UserContext.
/// </summary>
/// <returns>copy of UserAttributes.</returns>
UserAttributes GetAttributes();

/// <summary>
/// Returns Optimizely instance associated with the UserContext.
/// </summary>
/// <returns> Optimizely instance.</returns>
Optimizely GetOptimizely();

/// <summary>
/// Returns UserId associated with the UserContext
/// </summary>
/// <returns>UserId of this instance.</returns>
string GetUserId();

/// <summary>
/// Set an attribute for a given key.
/// </summary>
/// <param name="key">An attribute key</param>
/// <param name="value">value An attribute value</param>
void SetAttribute(string key, object value);

/// <summary>
/// Track an event.
/// </summary>
/// <param name="eventName">The event name.</param>
void TrackEvent(string eventName);

/// <summary>
/// Track an event.
/// </summary>
/// <param name="eventName">The event name.</param>
/// <param name="eventTags">A map of event tag names to event tag values.</param>
void TrackEvent(string eventName, EventTags eventTags);
}
}
8 changes: 4 additions & 4 deletions OptimizelySDK/Optimizely.cs
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ public OptimizelyJSON GetFeatureVariableJSON(string featureKey, string variableK
/// <param name="userId">The user ID to be used for bucketing.</param>
/// <param name="userAttributes">The user's attributes</param>
/// <returns>OptimizelyUserContext | An OptimizelyUserContext associated with this OptimizelyClient.</returns>
public OptimizelyUserContext CreateUserContext(string userId,
public IOptimizelyUserContext CreateUserContext(string userId,
UserAttributes userAttributes = null)
{
var inputValues = new Dictionary<string, string>
Expand All @@ -724,7 +724,7 @@ public OptimizelyUserContext CreateUserContext(string userId,
/// <param name="key">A flag key for which a decision will be made.</param>
/// <param name="options">A list of options for decision-making.</param>
/// <returns>A decision result.</returns>
internal OptimizelyDecision Decide(OptimizelyUserContext user,
internal OptimizelyDecision Decide(IOptimizelyUserContext user,
string key,
OptimizelyDecideOption[] options)
{
Expand Down Expand Up @@ -844,7 +844,7 @@ internal OptimizelyDecision Decide(OptimizelyUserContext user,
reasonsToReport);
}

internal Dictionary<string, OptimizelyDecision> DecideAll(OptimizelyUserContext user,
internal Dictionary<string, OptimizelyDecision> DecideAll(IOptimizelyUserContext user,
OptimizelyDecideOption[] options)
{
var decisionMap = new Dictionary<string, OptimizelyDecision>();
Expand All @@ -862,7 +862,7 @@ internal Dictionary<string, OptimizelyDecision> DecideAll(OptimizelyUserContext
return DecideForKeys(user, allFlagKeys, options);
}

internal Dictionary<string, OptimizelyDecision> DecideForKeys(OptimizelyUserContext user,
internal Dictionary<string, OptimizelyDecision> DecideForKeys(IOptimizelyUserContext user,
string[] keys,
OptimizelyDecideOption[] options)
{
Expand Down
6 changes: 3 additions & 3 deletions OptimizelySDK/OptimizelyDecisions/OptimizelyDecision.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class OptimizelyDecision
/// <summary>
/// user context for which the decision was made.
/// </summary>
public OptimizelyUserContext UserContext { get; private set; }
public IOptimizelyUserContext UserContext { get; private set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

and this one as well, please revert to OptimizelyUserContext

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 same as before


/// <summary>
/// an array of error/info/debug messages describing why the decision has been made.
Expand All @@ -65,7 +65,7 @@ public OptimizelyDecision(string variationKey,
OptimizelyJSON variables,
string ruleKey,
string flagKey,
OptimizelyUserContext userContext,
IOptimizelyUserContext userContext,
string[] reasons)
{
VariationKey = variationKey;
Expand All @@ -84,7 +84,7 @@ public OptimizelyDecision(string variationKey,
/// and error reason array
/// </summary>
public static OptimizelyDecision NewErrorDecision(string key,
OptimizelyUserContext optimizelyUserContext,
IOptimizelyUserContext optimizelyUserContext,
string error,
IErrorHandler errorHandler,
ILogger logger)
Expand Down
1 change: 1 addition & 0 deletions OptimizelySDK/OptimizelySDK.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
<Compile Include="Entity\Group.cs" />
<Compile Include="Entity\IdKeyEntity.cs" />
<Compile Include="Event\Entity\DecisionMetadata.cs" />
<Compile Include="IOptimizelyUserContext.cs" />
<Compile Include="OptimizelyDecisions\DecisionMessage.cs" />
<Compile Include="OptimizelyDecisions\DecisionReasons.cs" />
<Compile Include="OptimizelyDecisions\OptimizelyDecideOption.cs" />
Expand Down
5 changes: 3 additions & 2 deletions OptimizelySDK/OptimizelyUserContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace OptimizelySDK
/// <summary>
/// OptimizelyUserContext defines user contexts that the SDK will use to make decisions for
/// </summary>
public class OptimizelyUserContext
public class OptimizelyUserContext : IOptimizelyUserContext
{
private ILogger Logger;
private IErrorHandler ErrorHandler;
Expand All @@ -46,7 +46,7 @@ public OptimizelyUserContext(Optimizely optimizely, string userId, UserAttribute
UserId = userId;
}

private OptimizelyUserContext Copy() => new OptimizelyUserContext(Optimizely, UserId, GetAttributes(), ErrorHandler, Logger);
private IOptimizelyUserContext Copy() => new OptimizelyUserContext(Optimizely, UserId, GetAttributes(), ErrorHandler, Logger);

/// <summary>
/// Returns Optimizely instance associated with the UserContext.
Expand Down Expand Up @@ -133,6 +133,7 @@ public OptimizelyDecision Decide(string key,
/// Returns a key-map of decision results for multiple flag keys and a user context.
/// </summary>
/// <param name="keys">list of flag keys for which a decision will be made.</param>
/// <param name="options">An array of decision options.</param>
/// <returns>A dictionary of all decision results, mapped by flag keys.</returns>
public Dictionary<string, OptimizelyDecision> DecideForKeys(string[] keys, OptimizelyDecideOption[] options)
{
Expand Down