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

Fix GlobalCleanupAttributeTest.GlobalCleanupMethodRunsTest #894

Merged
merged 2 commits into from Oct 4, 2018

Conversation

dlemstra
Copy link
Contributor

@dlemstra dlemstra commented Oct 3, 2018

This PR fixes corrects the tests because this test will also pass when

[Benchmark]
public void Benchmark()
{
    Console.WriteLine(BenchmarkCalled);
}

is changed to

[Benchmark]
public void Benchmark()
{
    //Console.WriteLine(BenchmarkCalled);
}

This is happening because there is a check if log contains GlobalCleanupCalled instead BenchmarkCalled. When the log only contains GlobalCleanupCalled this will happen:

string log = logger.GetLog();
Assert.Contains(GlobalCleanupCalled + System.Environment.NewLine, log);
// True
Assert.True(
    log.IndexOf(GlobalCleanupCalled + System.Environment.NewLine) >
    log.IndexOf(BenchmarkCalled + System.Environment.NewLine));
// 0 > -1 == true

@AndreyAkinshin AndreyAkinshin self-requested a review October 4, 2018 08:49
Copy link
Member

@AndreyAkinshin AndreyAkinshin left a comment

Choose a reason for hiding this comment

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

LGTM

@AndreyAkinshin AndreyAkinshin added this to the v0.11.2 milestone Oct 4, 2018
@AndreyAkinshin AndreyAkinshin merged commit 382a4af into dotnet:master Oct 4, 2018
@AndreyAkinshin
Copy link
Member

@dlemstra, thank you!

@dlemstra
Copy link
Contributor Author

dlemstra commented Oct 4, 2018

@AndreyAkinshin What is going on with the tests on travis? Would you be interested in giving Azure DevOps Pipelines a try? I don't mind setting that up in another PR if you would like to have this added.

@dlemstra dlemstra deleted the fix-unit-test branch October 4, 2018 09:07
@AndreyAkinshin
Copy link
Member

What is going on with the tests on travis?

It seems that the OS image preparation takes too much time on Linux. Our Linux build is flaky because Travis often kills it because of the 50 min timeout. We don't have this problem on macOS (build takes about 30 min).
@Ky7m, is it possible to do something with it?

Would you be interested in giving Azure DevOps Pipelines a try? I don't mind setting that up in another PR if you would like to have this added.

It would be great!

@dlemstra
Copy link
Contributor Author

dlemstra commented Oct 4, 2018

@AndreyAkinshin Will give it a try this weekend.

@Ky7m
Copy link
Contributor

Ky7m commented Oct 4, 2018

@AndreyAkinshin yes, I've created an issue for that #875.
@dlemstra are you going to implement this?

@dlemstra
Copy link
Contributor Author

dlemstra commented Oct 4, 2018

@Ky7m I could implement that if you dont mind me doing that

@Ky7m
Copy link
Contributor

Ky7m commented Oct 4, 2018

@dlemstra it would be awesome, just let me know if you need any help from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants