From 370e98d559a810df06f29a8314ef0c97221c4eac Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Sat, 4 May 2019 07:33:21 -0700 Subject: [PATCH] System.Data.OleDb.Tests.OleDbConnectionTests.GetSchema failed in CI (dotnet/corefx#37438) Fixes dotnet/corefx#37411 - Remove small bit of dead code. - Add test and cleanup test code - Fix System.Data.OleDb.Tests.OleDbConnectionTests.GetSchema failure on CI Commit migrated from https://github.com/dotnet/corefx/commit/742d6367892167256617de5c3c7b9b1eae67bea2 --- .../System.Data.OleDb/src/OleDbDataReader.cs | 1 - src/libraries/System.Data.OleDb/src/OleDb_Enum.cs | 22 ------- .../System/Data/ProviderBase/DbMetaDataFactory.cs | 1 - src/libraries/System.Data.OleDb/tests/Helpers.cs | 1 + .../tests/OleDbCommandBuilderTests.cs | 32 ++++----- .../System.Data.OleDb/tests/OleDbCommandTests.cs | 12 ++++ .../tests/OleDbConnectionTests.cs | 48 ++++++-------- .../tests/OleDbDataAdapterTests.cs | 11 ++-- .../tests/OleDbDataReaderTests.cs | 75 ++++------------------ 9 files changed, 62 insertions(+), 141 deletions(-) diff --git a/src/libraries/System.Data.OleDb/src/OleDbDataReader.cs b/src/libraries/System.Data.OleDb/src/OleDbDataReader.cs index f46affc..4c16e74 100644 --- a/src/libraries/System.Data.OleDb/src/OleDbDataReader.cs +++ b/src/libraries/System.Data.OleDb/src/OleDbDataReader.cs @@ -1623,7 +1623,6 @@ namespace System.Data.OleDb short getType = info.type.wType; Debug.Assert(NativeDBType.STR != getType, "Should have bound as WSTR"); - Debug.Assert(!NativeDBType.HasHighBit(getType), "CreateAccessor - unexpected high bits on datatype"); if (-1 != info.size) { diff --git a/src/libraries/System.Data.OleDb/src/OleDb_Enum.cs b/src/libraries/System.Data.OleDb/src/OleDb_Enum.cs index 9e93e62..8c57594 100644 --- a/src/libraries/System.Data.OleDb/src/OleDb_Enum.cs +++ b/src/libraries/System.Data.OleDb/src/OleDb_Enum.cs @@ -88,28 +88,6 @@ namespace System.Data.OleDb // high mask internal const short HighMask = unchecked((short)0xf000); - static internal bool HasHighBit(short value) - { - return (0 != (HighMask & value)); - } - /* - static internal bool IsArray(short value) { - return (ARRAY == (HighMask & value)); - } - static internal bool IsByRef(short value) { - return (BYREF == (HighMask & value)); - } - static internal bool IsReserved(short value) { - return (RESERVED == (HighMask & value)); - } - static internal bool IsVector(short value) { - return (VECTOR == (HighMask & value)); - } - static internal int GetLowBits(short value) { - return (value & ~HighMask); - } - */ - private const string S_BINARY = "DBTYPE_BINARY"; // DBTYPE_BYTES private const string S_BOOL = "DBTYPE_BOOL"; private const string S_BSTR = "DBTYPE_BSTR"; diff --git a/src/libraries/System.Data.OleDb/src/System/Data/ProviderBase/DbMetaDataFactory.cs b/src/libraries/System.Data.OleDb/src/System/Data/ProviderBase/DbMetaDataFactory.cs index 69bed5c..e028e7b 100644 --- a/src/libraries/System.Data.OleDb/src/System/Data/ProviderBase/DbMetaDataFactory.cs +++ b/src/libraries/System.Data.OleDb/src/System/Data/ProviderBase/DbMetaDataFactory.cs @@ -416,7 +416,6 @@ namespace System.Data.ProviderBase { Debug.Assert(_metaDataCollectionsDataSet != null); - //TODO: MarkAsh or EnzoL should review this code for efficency. DataTable metaDataCollectionsTable = _metaDataCollectionsDataSet.Tables[DbMetaDataCollectionNames.MetaDataCollections]; DataColumn populationMechanismColumn = metaDataCollectionsTable.Columns[_populationMechanism]; DataColumn collectionNameColumn = metaDataCollectionsTable.Columns[DbMetaDataColumnNames.CollectionName]; diff --git a/src/libraries/System.Data.OleDb/tests/Helpers.cs b/src/libraries/System.Data.OleDb/tests/Helpers.cs index ffea07a..60ebce8 100644 --- a/src/libraries/System.Data.OleDb/tests/Helpers.cs +++ b/src/libraries/System.Data.OleDb/tests/Helpers.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Runtime.InteropServices; +using Xunit; namespace System.Data.OleDb.Tests { diff --git a/src/libraries/System.Data.OleDb/tests/OleDbCommandBuilderTests.cs b/src/libraries/System.Data.OleDb/tests/OleDbCommandBuilderTests.cs index 90cd021..680c953 100644 --- a/src/libraries/System.Data.OleDb/tests/OleDbCommandBuilderTests.cs +++ b/src/libraries/System.Data.OleDb/tests/OleDbCommandBuilderTests.cs @@ -28,12 +28,9 @@ namespace System.Data.OleDb.Tests using (var cmd = (OleDbCommand)OleDbFactory.Instance.CreateCommand()) { cmd.CommandType = commandType; - var exception = Record.Exception(() => OleDbCommandBuilder.DeriveParameters(cmd)); - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - string.Format("{0} DeriveParameters only supports CommandType.StoredProcedure, not CommandType.{1}.", nameof(OleDbCommand), cmd.CommandType.ToString()), - exception.Message); + AssertExtensions.Throws( + () => OleDbCommandBuilder.DeriveParameters(cmd), + $"{nameof(OleDbCommand)} DeriveParameters only supports CommandType.StoredProcedure, not CommandType.{cmd.CommandType.ToString()}."); } } @@ -43,13 +40,10 @@ namespace System.Data.OleDb.Tests using (var cmd = (OleDbCommand)OleDbFactory.Instance.CreateCommand()) { cmd.CommandType = CommandType.StoredProcedure; - cmd.CommandText = null; - var exception = Record.Exception(() => OleDbCommandBuilder.DeriveParameters(cmd)); - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - string.Format("{0}: {1} property has not been initialized", nameof(OleDbCommandBuilder.DeriveParameters), nameof(cmd.CommandText)), - exception.Message); + cmd.CommandText = null; + AssertExtensions.Throws( + () => OleDbCommandBuilder.DeriveParameters(cmd), + $"{nameof(OleDbCommandBuilder.DeriveParameters)}: {nameof(cmd.CommandText)} property has not been initialized"); } } @@ -63,12 +57,10 @@ namespace System.Data.OleDb.Tests cmd.CommandType = CommandType.StoredProcedure; cmd.CommandText = @"SELECT * FROM " + tableName; cmd.Connection = null; - var exception = Record.Exception(() => OleDbCommandBuilder.DeriveParameters(cmd)); - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - string.Format("{0}: {1} property has not been initialized.", nameof(OleDbCommandBuilder.DeriveParameters), nameof(cmd.Connection)), - exception.Message); + + AssertExtensions.Throws( + () => OleDbCommandBuilder.DeriveParameters(cmd), + $"{nameof(OleDbCommandBuilder.DeriveParameters)}: {nameof(cmd.Connection)} property has not been initialized."); } }); } @@ -88,7 +80,7 @@ namespace System.Data.OleDb.Tests Assert.NotNull(exception); Assert.IsType(exception); Assert.Contains( - string.Format("{0} requires an open and available Connection.", nameof(OleDbCommandBuilder.DeriveParameters)), + $"{nameof(OleDbCommandBuilder.DeriveParameters)} requires an open and available Connection.", exception.Message); } }); diff --git a/src/libraries/System.Data.OleDb/tests/OleDbCommandTests.cs b/src/libraries/System.Data.OleDb/tests/OleDbCommandTests.cs index 282c935..d20f339 100644 --- a/src/libraries/System.Data.OleDb/tests/OleDbCommandTests.cs +++ b/src/libraries/System.Data.OleDb/tests/OleDbCommandTests.cs @@ -13,6 +13,18 @@ namespace System.Data.OleDb.Tests { public class OleDbCommandTests : OleDbTestBase { + [ConditionalFact(Helpers.IsDriverAvailable)] + public void CommandType_SetInvalidValue_Throws() + { + using (var cmd = (OleDbCommand)OleDbFactory.Instance.CreateCommand()) + { + AssertExtensions.Throws( + () => cmd.CommandType = (CommandType)0, + $"The CommandType enumeration value, 0, is invalid.\r\nParameter name: {nameof(cmd.CommandType)}" + ); + } + } + [OuterLoop] [ConditionalFact(Helpers.IsDriverAvailable)] public void CommandType_InvalidType_Throws() diff --git a/src/libraries/System.Data.OleDb/tests/OleDbConnectionTests.cs b/src/libraries/System.Data.OleDb/tests/OleDbConnectionTests.cs index ed52272..85380ce 100644 --- a/src/libraries/System.Data.OleDb/tests/OleDbConnectionTests.cs +++ b/src/libraries/System.Data.OleDb/tests/OleDbConnectionTests.cs @@ -113,28 +113,25 @@ namespace System.Data.OleDb.Tests } } - [ConditionalFact(Helpers.IsDriverAvailable)] - public void GetSchema() + [ConditionalTheory(Helpers.IsDriverAvailable)] + [InlineData(nameof(DbMetaDataCollectionNames.MetaDataCollections), "CollectionName")] + [InlineData(nameof(DbMetaDataCollectionNames.DataSourceInformation), "CompositeIdentifierSeparatorPattern")] + [InlineData(nameof(DbMetaDataCollectionNames.DataTypes), "TypeName")] + public void GetSchema(string tableName, string columnName) { - using (var oleDbConnection = new OleDbConnection(ConnectionString)) - { - oleDbConnection.Open(); - - DataTable metaDataCollections = oleDbConnection.GetSchema(DbMetaDataCollectionNames.MetaDataCollections); - Assert.True(metaDataCollections != null && metaDataCollections.Rows.Count > 0); - - DataTable metaDataSourceInfo = oleDbConnection.GetSchema(DbMetaDataCollectionNames.DataSourceInformation); - Assert.True(metaDataSourceInfo != null && metaDataSourceInfo.Rows.Count > 0); - - DataTable metaDataTypes = oleDbConnection.GetSchema(DbMetaDataCollectionNames.DataTypes); - Assert.True(metaDataTypes != null && metaDataTypes.Rows.Count > 0); - - DataTable schema = oleDbConnection.GetSchema(); - Assert.True(schema != null && schema.Rows.Count > 0); - - Assert.Throws( - () => oleDbConnection.GetSchema(DbMetaDataCollectionNames.MetaDataCollections, new string[] { new string('a', 5000) } )); - } + DataTable schema = connection.GetSchema(tableName); + Assert.True(schema != null && schema.Rows.Count > 0); + var exception = Record.Exception(() => schema.Rows[0].Field(columnName)); + Assert.Null(exception); + + AssertExtensions.Throws( + () => connection.GetSchema(tableName, new string[] { null }), + $"More restrictions were provided than the requested schema ('{tableName}') supports." + ); + const string MissingColumn = "MissingColumn"; + AssertExtensions.Throws( + () => schema.Rows[0].Field>(MissingColumn), + $"Column '{MissingColumn}' does not belong to table {tableName}."); } [ConditionalFact(Helpers.IsDriverAvailable)] @@ -215,12 +212,9 @@ namespace System.Data.OleDb.Tests ConnectionString }.AsSpan(); File.WriteAllLines(udlFile, lines.Slice(start, length).ToArray()); - var exception = Record.Exception(() => new OleDbConnection(@"file name = " + udlFile)); - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - "Invalid UDL file.", - exception.Message); + AssertExtensions.Throws( + () => new OleDbConnection(@"file name = " + udlFile), + "Invalid UDL file."); } [ConditionalFact(Helpers.IsDriverAvailable)] diff --git a/src/libraries/System.Data.OleDb/tests/OleDbDataAdapterTests.cs b/src/libraries/System.Data.OleDb/tests/OleDbDataAdapterTests.cs index bbd2fb4..d564a81 100644 --- a/src/libraries/System.Data.OleDb/tests/OleDbDataAdapterTests.cs +++ b/src/libraries/System.Data.OleDb/tests/OleDbDataAdapterTests.cs @@ -132,18 +132,15 @@ namespace System.Data.OleDb.Tests Action FillShouldThrow = (shouldFail) => { DataSet ds = new DataSet(); OleDbDataAdapter adapter = new OleDbDataAdapter(command); - var exception = Record.Exception(() => adapter.Fill(ds, tableName)); if (shouldFail) { - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - "There is already an open DataReader associated with this Command which must be closed first.", - exception.Message); + AssertExtensions.Throws( + () => adapter.Fill(ds, tableName), + "There is already an open DataReader associated with this Command which must be closed first."); } else { - Assert.Null(exception); + Assert.NotNull(adapter.Fill(ds, tableName)); } }; using (var reader = command.ExecuteReader()) diff --git a/src/libraries/System.Data.OleDb/tests/OleDbDataReaderTests.cs b/src/libraries/System.Data.OleDb/tests/OleDbDataReaderTests.cs index e73f994..ca8ef44 100644 --- a/src/libraries/System.Data.OleDb/tests/OleDbDataReaderTests.cs +++ b/src/libraries/System.Data.OleDb/tests/OleDbDataReaderTests.cs @@ -32,12 +32,9 @@ namespace System.Data.OleDb.Tests Assert.True(reader.HasRows); DataTable schema = reader.GetSchemaTable(); Assert.Equal(4, schema.Rows.Count); - var exception = Record.Exception(() => reader.GetString(5)); - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - "Index was outside the bounds of the array.", - exception.Message); + AssertExtensions.Throws( + () => reader.GetString(5), + "Index was outside the bounds of the array."); }); } @@ -48,12 +45,9 @@ namespace System.Data.OleDb.Tests RunTest((reader) => { reader.Read(); Assert.True(reader.HasRows); - var exception = Record.Exception(() => reader["NonExistentColumn"]); - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - "NonExistentColumn", - exception.Message); + object obj; + AssertExtensions.Throws( + () => obj = reader["NonExistentColumn"], "NonExistentColumn"); }); } @@ -76,23 +70,13 @@ namespace System.Data.OleDb.Tests [ConditionalFact(Helpers.IsDriverAvailable)] public void EmptyReader_SchemaOnly_EmptyReader() { + const string expectedMessage = "No data exists for the row/column."; RunTest((reader) => { reader.Read(); Assert.False(reader.HasRows); - var exception = Record.Exception(() => reader.GetString(1)); - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - "No data exists for the row/column.", - exception.Message); - - var values = new object[1]; - exception = Record.Exception(() => reader.GetValues(values)); - Assert.NotNull(exception); - Assert.IsType(exception); - Assert.Equal( - "No data exists for the row/column.", - exception.Message); + AssertExtensions.Throws(() => reader.GetString(1), expectedMessage); + AssertExtensions.Throws(() => reader.GetValues(new object[1]), expectedMessage); + AssertExtensions.Throws(() => reader.GetData(0), expectedMessage); }, schemaOnly: true); } @@ -207,51 +191,16 @@ namespace System.Data.OleDb.Tests [OuterLoop] [ConditionalFact(Helpers.IsDriverAvailable)] - public void Depth_IsClosed_Throws() + public void IsClosed_CallReaderApis_Throws() { RunTest((reader) => { reader.Close(); Assert.Throws(() => reader.Depth); - }); - } - - [OuterLoop] - [ConditionalFact(Helpers.IsDriverAvailable)] - public void FieldCount_IsClosed_Throws() - { - RunTest((reader) => { - reader.Close(); Assert.Throws(() => reader.FieldCount); - }); - } - - [OuterLoop] - [ConditionalFact(Helpers.IsDriverAvailable)] - public void VisibleFieldCount_IsClosed_Throws() - { - RunTest((reader) => { - reader.Close(); Assert.Throws(() => reader.VisibleFieldCount); - }); - } - - [OuterLoop] - [ConditionalFact(Helpers.IsDriverAvailable)] - public void HasRows_IsClosed_Throws() - { - RunTest((reader) => { - reader.Close(); Assert.Throws(() => reader.HasRows); - }); - } - - [OuterLoop] - [ConditionalFact(Helpers.IsDriverAvailable)] - public void GetSchemaTable_IsClosed_Throws() - { - RunTest((reader) => { - reader.Close(); Assert.Throws(() => reader.GetSchemaTable()); + Assert.Throws(() => reader.NextResult()); }); } -- 2.7.4