Skip to content

Clean up db tests dependencies #1401

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

Merged

Conversation

fredericDelaporte
Copy link
Member

@fredericDelaporte fredericDelaporte commented Oct 24, 2017

Aims at simplifying dependencies management.

Use NuGet for database tests when possible

  • FirebirdSql.Data.FirebirdClient
  • Microsoft.SqlServer.Compact
  • MySql.Data
  • Npgsql (3.2.4.1, due to 3.2.5 no more supporting NHibernate closing connection from distributed transaction second phase: maybe we should add a condition for not testing that instead)
  • Oracle.ManagedDataAccess (12.1.2400, newer versions (at least up to 12.2.1100) fail NH-1171 tests, apparently due to some old Oracle bug striking back https://stackoverflow.com/a/7542670/1178314 )
  • System.Data.SQLite.Core

Folder available-test-configurations of NH devs boxes need to be cleaned accordingly. (Remove binaries matching those packages.)

@@ -19,6 +19,7 @@
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateConstants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateInstanceFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /&gt;</s:String>
<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpAttributeForSingleLineMethodUpgrade/@EntryIndexedValue">True</s:Boolean>
Copy link
Member Author

Choose a reason for hiding this comment

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

Rider insists at doing that since its last update.

set LIB_FILES=
set LIB_FILES2=
goto test-setup-generic

:test-setup-sqlservercex86
set CONFIG_NAME=SqlServerCe32
set PLATFORM=AnyCPU
set TEST_PLATFORM=AnyCPU
set LIB_FILES=lib\teamcity\SqlServerCe\*.dll
set LIB_FILES2=lib\teamcity\SqlServerCe\X86\*.dll
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove these lines for the SQL Server Compact native files by linking to the files from the test .csproj, and copying to output, and also including a PackageReference to Microsoft.SqlServer.Compact.

Alternatively, a custom .target file can be used to pull the files from the $(NuGetPackageRoot) and copy them to output.

Copy link
Contributor

@ngbrown ngbrown Oct 25, 2017

Choose a reason for hiding this comment

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

Never mind about the .target, that's just how I automate the whole thing for subsequent dependencies. For just a single test project, include this:

  <ItemGroup>
    <PackageReference Include="Microsoft.SqlServer.Compact" Version="4.0.8876.1" />
  </ItemGroup>
  <ItemGroup Condition=" '$(NuGetPackageRoot)' != '' ">
      <NativeBinaries Include="$(NuGetPackageRoot)microsoft.sqlserver.compact\4.0.8876.1\NativeBinaries\**\*.*" />
      <Content Include="@(NativeBinaries)">
        <Link>%(RecursiveDir)%(Filename)%(Extension)</Link>
        <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      </Content>
  </ItemGroup>

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks,

Added to NHibernate.Test.csproj, seems taken into account by NHibernate.TestDatabaseSetup.csproj too, so it should fix last CE failure.

@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented Oct 25, 2017

Npgsql 3.2.5 fails tests involving closing the connection from second phase when the transaction is distributed. Outside of this PR, we use 3.2.2. I have checked all intermediate versions, they all work in this case, only 3.2.5 (current latest) fails. So upgrading to 3.2.4.1 instead.

The 3.2.5 failure may lead to pending prepared transaction on PostgreSQL db, causing it to prevent nhibernate database drop, wrecking all subsequent builds whatever the PR. It is then required to use the pgadmin tool on build agent for rollbacking pending prepared transactions (see here) and dropping the db.

@fredericDelaporte
Copy link
Member Author

Testing SqlServerCe NuGet package (I was falsely presuming it was not having an official one) and Oracle Managed driver.
If all work, this will mean all currently running teamcity builds will rely on NuGet rather than on manually deployed binaries. (Not counting the Oracle non managed build since it is suspended.)

@fredericDelaporte fredericDelaporte force-pushed the CleanUpDbTests branch 2 times, most recently from 0ad7a50 to 9ba83a2 Compare October 26, 2017 09:04
@fredericDelaporte
Copy link
Member Author

fredericDelaporte commented Oct 26, 2017

As said on another PR too, Oracle.ManagedDataAccess versions greater than 12.1.2400 (at least up to current 12.2.1100) fail NH-1171 tests, it seems it has its own bug with comments in SQL. And it is a known bug since quite long as shown here, but apparently with some versions suffering less of it. So just using its NuGet package without upgrading it.

@hazzik
Copy link
Member

hazzik commented Oct 26, 2017

I have a slightly different view on who and how shall reference these packages, but I think it is good for now.

@fredericDelaporte
Copy link
Member Author

I agree my approach is a bit "brute force" approach. Works for now, till no data providers have conflicting requirements.

@fredericDelaporte
Copy link
Member Author

Rebased.

@hazzik hazzik added this to the 5.1 milestone Nov 5, 2017
hazzik
hazzik previously approved these changes Nov 5, 2017
 * Creating a test configuration was setting PLATFORM, which MsBuild then take into account, wrecking some build options like async code generation.
 * It is no more needed to reference packages of other references
 * FirebirdSql.Data.FirebirdClient
 * Microsoft.SqlServer.Compact
 * MySql.Data
 * Npgsql (3.2.4.1, due to 3.2.5 no more supporting NHibernate closing connection from distributed transaction second phase: maybe we should add a condition for not testing that instead)
 * Oracle.ManagedDataAccess (12.1.2400, newer versions (at least up to 12.2.1100) fail NH-1171 tests, apparently due to some old Oracle bug striking back https://stackoverflow.com/a/7542670/1178314 )
 * System.Data.SQLite.Core

Folder available-test-configurations of NH devs need to be cleaned accordingly. (Remove binaries matching those packages.)
@fredericDelaporte fredericDelaporte merged commit ba58fdc into nhibernate:master Nov 6, 2017
@fredericDelaporte fredericDelaporte deleted the CleanUpDbTests branch November 6, 2017 09:33
@fredericDelaporte fredericDelaporte modified the milestones: 5.1, 5.0.1 Nov 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants