From b9dcf08d606be3924e2e438417230801bc756a20 Mon Sep 17 00:00:00 2001 From: Marinovsky Date: Tue, 2 Jan 2024 13:15:44 -0500 Subject: [PATCH] Fix bugs in unit tests --- Engine/Setup/BrokerageSetupHandler.cs | 27 +-- .../Setup/BrokerageSetupHandlerTests.cs | 175 +++++++----------- 2 files changed, 78 insertions(+), 124 deletions(-) diff --git a/Engine/Setup/BrokerageSetupHandler.cs b/Engine/Setup/BrokerageSetupHandler.cs index 509923e42b5c..6c85fee785cf 100644 --- a/Engine/Setup/BrokerageSetupHandler.cs +++ b/Engine/Setup/BrokerageSetupHandler.cs @@ -79,7 +79,7 @@ public class BrokerageSetupHandler : ISetupHandler // saves ref to algo so we can call quit if runtime error encountered private IBrokerageFactory _factory; private IBrokerage _dataQueueHandlerBrokerage; - private HashSet _supportedSecurityTypes = new() + protected virtual HashSet SupportedSecurityTypes => new() { SecurityType.Equity, SecurityType.Forex, @@ -394,7 +394,7 @@ protected bool LoadExistingHoldingsAndOrders(IBrokerage brokerage, IAlgorithm al Log.Trace("BrokerageSetupHandler.Setup(): Fetching open orders from brokerage..."); try { - GetOpenOrders(algorithm, parameters.ResultHandler, parameters.TransactionHandler, brokerage, _supportedSecurityTypes); + GetOpenOrders(algorithm, parameters.ResultHandler, parameters.TransactionHandler, brokerage); } catch (Exception err) { @@ -418,12 +418,9 @@ protected bool LoadExistingHoldingsAndOrders(IBrokerage brokerage, IAlgorithm al // verify existing holding security type Security security; - if (!algorithm.Securities.TryGetValue(holding.Symbol, out security)) + if (!algorithm.Securities.TryGetValue(holding.Symbol, out security) && !AddUnrequestedSecurity(algorithm, holding.Symbol, holding.Type, out security)) { - if (!AddUnrequestedSecurity(algorithm, holding.Symbol, holding.Type, out security)) - { - continue; - } + continue; } var exchangeTime = utcNow.ConvertFromUtc(security.Exchange.TimeZone); @@ -466,11 +463,11 @@ private bool AddUnrequestedSecurity(IAlgorithm algorithm, Symbol symbol, Securit { if (!algorithm.Securities.TryGetValue(symbol, out security)) { - if (!_supportedSecurityTypes.Contains((SecurityType)securityType)) + if (!SupportedSecurityTypes.Contains((SecurityType)securityType)) { Log.Error("BrokerageSetupHandler.Setup(): Unsupported security type: " + securityType + "-" + symbol.Value); AddInitializationError("Found unsupported security type in existing brokerage holdings: " + securityType + ". " + - "QuantConnect currently supports the following security types: " + string.Join(",", _supportedSecurityTypes)); + "QuantConnect currently supports the following security types: " + string.Join(",", SupportedSecurityTypes)); security = null; return false; } @@ -516,9 +513,7 @@ private bool AddUnrequestedSecurity(IAlgorithm algorithm, Symbol symbol, Securit /// The configured result handler /// The configurated transaction handler /// Brokerage output instance - /// The list of supported security types - protected void GetOpenOrders(IAlgorithm algorithm, IResultHandler resultHandler, ITransactionHandler transactionHandler, IBrokerage brokerage, - HashSet supportedSecurityTypes) + protected void GetOpenOrders(IAlgorithm algorithm, IResultHandler resultHandler, ITransactionHandler transactionHandler, IBrokerage brokerage) { // populate the algorithm with the account's outstanding orders var openOrders = brokerage.GetOpenOrders(); @@ -528,13 +523,9 @@ protected void GetOpenOrders(IAlgorithm algorithm, IResultHandler resultHandler, { // verify existing holding security type Security security; - if (!algorithm.Securities.TryGetValue(order.Symbol, out security)) + if (!algorithm.Securities.TryGetValue(order.Symbol, out security) && !AddUnrequestedSecurity(algorithm, order.Symbol, order.SecurityType, out security)) { - if (!AddUnrequestedSecurity(algorithm, order.Symbol, order.SecurityType, out security)) - { - // keep aggregating these errors - continue; - } + continue; } transactionHandler.AddOpenOrder(order, algorithm); diff --git a/Tests/Engine/Setup/BrokerageSetupHandlerTests.cs b/Tests/Engine/Setup/BrokerageSetupHandlerTests.cs index eed26ed72dbf..d14f3ffd4a58 100644 --- a/Tests/Engine/Setup/BrokerageSetupHandlerTests.cs +++ b/Tests/Engine/Setup/BrokerageSetupHandlerTests.cs @@ -37,6 +37,7 @@ using QuantConnect.Tests.Engine.DataFeeds; using QuantConnect.Util; using QuantConnect.Algorithm.CSharp; +using System.Collections; namespace QuantConnect.Tests.Engine.Setup { @@ -117,19 +118,19 @@ public void CanGetOpenOrders() Assert.Contains(Symbols.SPY, _algorithm.Securities.Select(x => x.Key).ToList()); } - [Test, TestCaseSource(nameof(GetExistingHoldingsAndOrdersTestCaseData))] + [TestCaseSource(typeof(ExistingHoldingAndOrdersDataClass), nameof(ExistingHoldingAndOrdersDataClass.GetExistingHoldingsAndOrdersTestCaseData))] public void SecondExistingHoldingsAndOrdersResolution(Func> getHoldings, Func> getOrders, bool expected) { ExistingHoldingsAndOrdersResolution(getHoldings, getOrders, expected, Resolution.Second); } - [Test, TestCaseSource(nameof(GetExistingHoldingsAndOrdersTestCaseData))] + [TestCaseSource(typeof(ExistingHoldingAndOrdersDataClass), nameof(ExistingHoldingAndOrdersDataClass.GetExistingHoldingsAndOrdersTestCaseData))] public void MinuteExistingHoldingsAndOrdersResolution(Func> getHoldings, Func> getOrders, bool expected) { ExistingHoldingsAndOrdersResolution(getHoldings, getOrders, expected, Resolution.Minute); } - [Test, TestCaseSource(nameof(GetExistingHoldingsAndOrdersTestCaseData))] + [TestCaseSource(typeof(ExistingHoldingAndOrdersDataClass), nameof(ExistingHoldingAndOrdersDataClass.GetExistingHoldingsAndOrdersTestCaseData))] public void TickExistingHoldingsAndOrdersResolution(Func> getHoldings, Func> getOrders, bool expected) { ExistingHoldingsAndOrdersResolution(getHoldings, getOrders, expected, Resolution.Tick); @@ -169,7 +170,7 @@ public void ExistingHoldingsAndOrdersResolution(Func> getHoldings, } } - [Test, TestCaseSource(nameof(GetExistingHoldingsAndOrdersTestCaseData))] + [TestCaseSource(typeof(ExistingHoldingAndOrdersDataClass), nameof(ExistingHoldingAndOrdersDataClass.GetExistingHoldingsAndOrdersTestCaseData))] public void ExistingHoldingsAndOrdersUniverseSettings(Func> getHoldings, Func> getOrders, bool expected) { // Set our universe settings @@ -232,34 +233,12 @@ public void ExistingHoldingsAndOrdersUniverseSettings(Func> getHol } } - [Test, TestCaseSource(nameof(GetExistingHoldingsAndOrdersTestCaseData))] + [TestCaseSource(typeof(ExistingHoldingAndOrdersDataClass),nameof(ExistingHoldingAndOrdersDataClass.GetExistingHoldingsAndOrdersTestCaseData))] public void LoadsExistingHoldingsAndOrders(Func> getHoldings, Func> getOrders, bool expected) { var algorithm = new TestAlgorithm(); algorithm.SetHistoryProvider(new BrokerageTransactionHandlerTests.BrokerageTransactionHandlerTests.EmptyHistoryProvider()); - var job = GetJob(); - - var resultHandler = new Mock(); - var transactionHandler = new Mock(); - var realTimeHandler = new Mock(); - var objectStore = new Mock(); - var brokerage = new Mock(); - - brokerage.Setup(x => x.IsConnected).Returns(true); - brokerage.Setup(x => x.AccountBaseCurrency).Returns(Currencies.USD); - brokerage.Setup(x => x.GetCashBalance()).Returns(new List()); - brokerage.Setup(x => x.GetAccountHoldings()).Returns(getHoldings); - brokerage.Setup(x => x.GetOpenOrders()).Returns(getOrders); - - var setupHandler = new BrokerageSetupHandler(); - - IBrokerageFactory factory; - setupHandler.CreateBrokerage(job, algorithm, out factory); - - var result = setupHandler.Setup(new SetupHandlerParameters(_dataManager.UniverseSelection, algorithm, brokerage.Object, job, resultHandler.Object, - transactionHandler.Object, realTimeHandler.Object, TestGlobals.DataCacheProvider, TestGlobals.MapFileProvider)); - - Assert.AreEqual(expected, result); + TestLoadExistingHoldingsAndOrders(algorithm, getHoldings, getOrders, expected); foreach (var security in algorithm.Securities.Values) { @@ -279,30 +258,7 @@ public void LoadsExistingHoldingsAndOrdersWithCustomData(Func> get var algorithm = new TestAlgorithm(); algorithm.AddData("BTC"); algorithm.SetHistoryProvider(new BrokerageTransactionHandlerTests.BrokerageTransactionHandlerTests.EmptyHistoryProvider()); - var job = GetJob(); - - var resultHandler = new Mock(); - var transactionHandler = new Mock(); - var realTimeHandler = new Mock(); - var objectStore = new Mock(); - var brokerage = new Mock(); - - brokerage.Setup(x => x.IsConnected).Returns(true); - brokerage.Setup(x => x.AccountBaseCurrency).Returns(Currencies.USD); - brokerage.Setup(x => x.GetCashBalance()).Returns(new List()); - brokerage.Setup(x => x.GetAccountHoldings()).Returns(getHoldings); - brokerage.Setup(x => x.GetOpenOrders()).Returns(getOrders); - - var setupHandler = new TestBrokerageSetupHandler(); - - IBrokerageFactory factory; - setupHandler.CreateBrokerage(job, algorithm, out factory); - - var parameters = new SetupHandlerParameters(_dataManager.UniverseSelection, algorithm, brokerage.Object, job, resultHandler.Object, - transactionHandler.Object, realTimeHandler.Object, TestGlobals.DataCacheProvider, TestGlobals.MapFileProvider); - var result = setupHandler.TestLoadExistingHoldingsAndOrders(brokerage.Object, algorithm, parameters); - - Assert.AreEqual(expected, result); + TestLoadExistingHoldingsAndOrders(algorithm, getHoldings, getOrders, expected); } [TestCase(true)] @@ -626,6 +582,34 @@ public void HasErrorWithZeroTotalPortfolioValue(bool hasCashBalance, bool hasHol } } + private void TestLoadExistingHoldingsAndOrders(IAlgorithm algorithm, Func> getHoldings, Func> getOrders, bool expected) + { + var job = GetJob(); + + var resultHandler = new Mock(); + var transactionHandler = new Mock(); + var realTimeHandler = new Mock(); + var objectStore = new Mock(); + var brokerage = new Mock(); + + brokerage.Setup(x => x.IsConnected).Returns(true); + brokerage.Setup(x => x.AccountBaseCurrency).Returns(Currencies.USD); + brokerage.Setup(x => x.GetCashBalance()).Returns(new List()); + brokerage.Setup(x => x.GetAccountHoldings()).Returns(getHoldings); + brokerage.Setup(x => x.GetOpenOrders()).Returns(getOrders); + + var setupHandler = new TestBrokerageSetupHandler(); + + IBrokerageFactory factory; + setupHandler.CreateBrokerage(job, algorithm, out factory); + + var parameters = new SetupHandlerParameters(_dataManager.UniverseSelection, algorithm, brokerage.Object, job, resultHandler.Object, + transactionHandler.Object, realTimeHandler.Object, TestGlobals.DataCacheProvider, TestGlobals.MapFileProvider); + var result = setupHandler.Setup(new SetupHandlerParameters(_dataManager.UniverseSelection, algorithm, brokerage.Object, job, resultHandler.Object, + transactionHandler.Object, realTimeHandler.Object, TestGlobals.DataCacheProvider, TestGlobals.MapFileProvider)); + Assert.AreEqual(expected, result); + } + private static object[] GetExistingHoldingsAndOrdersWithCustomDataTestCase = { new object[] { @@ -639,17 +623,17 @@ public void HasErrorWithZeroTotalPortfolioValue(bool hasCashBalance, bool hasHol true } }; - private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() + private class ExistingHoldingAndOrdersDataClass { - return new[] + public static IEnumerable GetExistingHoldingsAndOrdersTestCaseData { - new TestCaseData( - new Func>(() => new List()), - new Func>(() => new List()), - true) - .SetName("None"), + get + { + yield return new TestCaseData( + new Func>(() => new List()), + new Func>(() => new List()), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbols.SPY, Quantity = 1 } @@ -657,11 +641,9 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() new Func>(() => new List { new LimitOrder(Symbols.SPY, 1, 1, DateTime.UtcNow) - }), - true) - .SetName("Equity"), + }), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbols.SPY_C_192_Feb19_2016, Quantity = 1 } @@ -669,11 +651,9 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() new Func>(() => new List { new LimitOrder(Symbols.SPY_C_192_Feb19_2016, 1, 1, DateTime.UtcNow) - }), - true) - .SetName("Option"), + }), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbols.SPY, Quantity = 1 }, @@ -683,11 +663,9 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() { new LimitOrder(Symbols.SPY, 1, 1, DateTime.UtcNow), new LimitOrder(Symbols.SPY_C_192_Feb19_2016, 1, 1, DateTime.UtcNow) - }), - true) - .SetName("Equity + Option"), + }), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbols.SPY_C_192_Feb19_2016, Quantity = 1 }, @@ -697,11 +675,9 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() { new LimitOrder(Symbols.SPY_C_192_Feb19_2016, 1, 1, DateTime.UtcNow), new LimitOrder(Symbols.SPY, 1, 1, DateTime.UtcNow) - }), - true) - .SetName("Option + Equity"), + }), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbols.SPY_C_192_Feb19_2016, Quantity = 1 } @@ -709,11 +685,9 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() new Func>(() => new List { new LimitOrder(Symbols.SPY, 1, 1, DateTime.UtcNow), - }), - true) - .SetName("Equity open order + Option holding"), + }), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbols.EURUSD, Quantity = 1 } @@ -721,11 +695,9 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() new Func>(() => new List { new LimitOrder(Symbols.EURUSD, 1, 1, DateTime.UtcNow) - }), - true) - .SetName("Forex"), + }), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbols.BTCUSD, Quantity = 1 } @@ -733,11 +705,9 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() new Func>(() => new List { new LimitOrder(Symbols.BTCUSD, 1, 1, DateTime.UtcNow) - }), - true) - .SetName("Crypto"), + }), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbols.Fut_SPY_Feb19_2016, Quantity = 1 } @@ -745,11 +715,9 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() new Func>(() => new List { new LimitOrder(Symbols.Fut_SPY_Feb19_2016, 1, 1, DateTime.UtcNow) - }), - true) - .SetName("Future"), + }), true); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List { new Holding { Symbol = Symbol.Create("XYZ", SecurityType.Base, Market.USA), Quantity = 1 } @@ -757,22 +725,17 @@ private static TestCaseData[] GetExistingHoldingsAndOrdersTestCaseData() new Func>(() => new List { new LimitOrder("XYZ", 1, 1, DateTime.UtcNow) - }), - false) - .SetName("Base"), + }), false); - new TestCaseData( + yield return new TestCaseData( new Func>(() => { throw new Exception(); }), - new Func>(() => new List()), - false) - .SetName("Invalid Holdings"), + new Func>(() => new List()), false); - new TestCaseData( + yield return new TestCaseData( new Func>(() => new List()), - new Func>(() => { throw new Exception(); }), - false) - .SetName("Invalid Orders"), - }; + new Func>(() => { throw new Exception(); }), false); + } + } } internal class TestAlgorithm : QCAlgorithm @@ -825,14 +788,14 @@ public override void DebugMessage(string message) private class TestableBrokerageSetupHandler : BrokerageSetupHandler { - private readonly HashSet _supportedSecurityTypes = new HashSet + protected override HashSet SupportedSecurityTypes => new HashSet { SecurityType.Equity, SecurityType.Forex, SecurityType.Cfd, SecurityType.Option, SecurityType.Future, SecurityType.Crypto }; public void PublicGetOpenOrders(IAlgorithm algorithm, IResultHandler resultHandler, ITransactionHandler transactionHandler, IBrokerage brokerage) { - GetOpenOrders(algorithm, resultHandler, transactionHandler, brokerage, _supportedSecurityTypes); + GetOpenOrders(algorithm, resultHandler, transactionHandler, brokerage); } } }