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

Initialize FatalError handlers in VS and OOP #47554

Merged
merged 8 commits into from
Sep 19, 2020

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Sep 9, 2020

  • Initialize FatalError handlers in VS and OOP
  • Fail integration tests immediately on command execution failure

@sharwell sharwell requested a review from a team as a code owner September 9, 2020 17:50
@sharwell
Copy link
Member Author

sharwell commented Sep 9, 2020

First try didn't work as expected; trying again...

@sharwell sharwell force-pushed the disallow-fail branch 2 times, most recently from c7cd3cf to eab0555 Compare September 10, 2020 00:32
@sharwell sharwell changed the base branch from master to release/dev16.8 September 10, 2020 00:33
@sharwell sharwell changed the title Fail integration tests immediately on command execution failure Initialize FatalError handlers in VS and OOP Sep 10, 2020
Copy link
Member

@jaredpar jaredpar left a comment

Choose a reason for hiding this comment

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

This is removing one of the key parts of the compiler verification in order to enable telemetry in CI. That is not the right trade off here. We should make a more targeted disabling of this rather than whole sale disable it for all tests.

@sharwell
Copy link
Member Author

enable telemetry in CI

Enabling telemetry in CI is a non-goal for this PR. Our intent is to enable telemetry for shipping. I'm certainly open to alternatives to fix this.

@jaredpar
Copy link
Member

i'm certainly open to alternatives to fix this.

Disable the test that hit the assert.

@sharwell
Copy link
Member Author

@jaredpar Switched to just skip that test. May need to skip more depending on how the tests go.

This listener conflicts with ThrowingTraceListener, and significantly
impairs the ability to identify test failures.
Currently the incremental parser is not producing trees with the correct
text. However, this bug was introduced in the past and is failing now
that debug assertions are checked in builds.

See dotnet#47610
@333fred
Copy link
Member

333fred commented Sep 15, 2020

@sharwell what is this waiting on?

@sharwell
Copy link
Member Author

@333fred it still has a failing test (and possible crash), plus it needs ship room approval after it's passing

@tmat tmat changed the base branch from release/dev16.8 to master September 18, 2020 00:03
@sharwell sharwell merged commit ffa4e6d into dotnet:master Sep 19, 2020
@ghost ghost modified the milestones: 16.8.P3, Next Sep 19, 2020
@sharwell sharwell deleted the disallow-fail branch September 19, 2020 23:38
@dibarbet dibarbet modified the milestones: Next, 16.8.P4 Sep 21, 2020
dibarbet added a commit to dibarbet/roslyn that referenced this pull request Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants