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

Refactor/test version config #3649

Merged
merged 4 commits into from
Apr 17, 2019
Merged

Refactor/test version config #3649

merged 4 commits into from
Apr 17, 2019

Conversation

Mpdreamz
Copy link
Member

@Mpdreamz Mpdreamz commented Apr 8, 2019

  • Simplifies ITestConfiguration and its implementations.
  • Default version resolutions is now driven by the yaml file rather than also having baked in fallback in code.

Copy link
Contributor

@russcam russcam left a comment

Choose a reason for hiding this comment

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

LGTM. Just left a small comment about std redirection

@@ -54,7 +53,10 @@ module Tests =
| (true) -> [ "--logger"; "trx"; "--collect"; "\"Code Coverage\""; "-v"; "m"] |> List.append command
| _ -> command

Tooling.DotNet.ExecInWithTimeout "src/Tests/Tests" commandWithCodeCoverage (TimeSpan.FromMinutes 30.)
// using std redirection since that foces vstest not to redirect Console.Write from Elastic.Xunit somehow
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this comment be updated with how vstest and xunit communicate?

Copy link
Member Author

Choose a reason for hiding this comment

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

reverting this since Proc is not happy on linux

/// </summary>
public bool ShowElasticsearchOutputAfterStarted { get; } = true;

/// <summary> When specified will only run one overload in API tests, helpful when debugging locally </summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

comments 💯

@Mpdreamz Mpdreamz self-assigned this Apr 16, 2019
…starting the tests this fix's vstest breaking console.writeline though I am not quite sure yet as to why
@Mpdreamz Mpdreamz force-pushed the refactor/test-version-config branch from 41511e1 to 138a0b0 Compare April 16, 2019 13:06
…ixed launching of cluster on command line not doing any bootstrap.

Made environment config mode decision explicit rather then inferred based of NEST_INTEGRATION_VERSION
@Mpdreamz Mpdreamz merged commit 5403c8a into 7.x Apr 17, 2019
@Mpdreamz Mpdreamz deleted the refactor/test-version-config branch April 17, 2019 12:08
Mpdreamz added a commit that referenced this pull request Apr 17, 2019
* small fixes to how default versions are resolved and test configuration was a tad too complicated

* refactor ITestConfiguration a bit further, use console redirect when starting the tests this fix's vstest breaking console.writeline though I am not quite sure yet as to why

* Execute tests without standard redirection for now

* AssertStatusCode called base.AssertResponse on SearchUsageTestBase, fixed launching of cluster on command line not doing any bootstrap.

Made environment config mode decision explicit rather then inferred based of NEST_INTEGRATION_VERSION

(cherry picked from commit 5403c8a)
@Mpdreamz
Copy link
Member Author

ported to master

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.

2 participants