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

Run automated tests on debug builds #11632

Open
halter73 opened this issue Jun 27, 2019 · 10 comments
Open

Run automated tests on debug builds #11632

halter73 opened this issue Jun 27, 2019 · 10 comments
Labels
affected-few This issue impacts only small number of customers area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework severity-minor This label is used by an internal tool task

Comments

@halter73
Copy link
Member

halter73 commented Jun 27, 2019

My understanding is that our CI servers now only run automated tests against release builds. If that's the case, this means that the many Debug.Asserts throughout the AspNetCore repo are pretty worthless since we can only ever see an assertion failures by running tests or samples locally even the Debug.Asserts in the test projects themselves.

I think the best solution is to run our automated tests against debug builds. I'm not asking for all our testing to be done against debug builds, but nightly tests of debug builds would probably be a good idea.

@davidfowl suggested using critical logs to replace Debug.Asserts. That was definitely the right approach for #11624, but in a lot of places we assert currently we don't have a logger. There's also the problem that not all our tests will fail given a critical log, but a lot will.

Changing Debug.Asserts to Trace.Asserts seems like a somewhat reasonable idea, but not only does that create performance concerns, it could also potentially introduce new DoS vectors as @benaadams rightfully pointed out.

@halter73 halter73 added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jun 27, 2019
@benaadams
Copy link
Member

Run automated tests on debug builds

I'd go for this option; at least one (e.g. Windows); preferably two (e.g. Windows and Linux); while keeping the current release testing that runs.

@analogrelay
Copy link
Contributor

analogrelay commented Jun 27, 2019

I think the best solution is to run our automated tests against debug builds.

This is also relatively easy to do in parallel by simply adding jobs to the YAML of DOOM. It theoretically would magnify existing flakiness slightly by giving flaky tests and extra couple chances to fail, but we need to drive those down anyway.

We should also make sure we know the behavior and diagnosability of Debug.Assert in our test code. In the past it's failed in hard-to-understand ways ("Test Run Aborted" errors, etc.).

suggested using critical logs to replace Debug.Asserts.

Why not both? We could have a helper that writes a critical log (for Release) and does a Debug.Assert (for Debug).

@BrennanConroy
Copy link
Member

We run on release and debug in Extensions so there is prior art
https://github.com/aspnet/Extensions/blob/8c6d69570722ee4cd6583cbb4af2a9c74c8c10dc/azure-pipelines.yml#L148

@halter73
Copy link
Member Author

@anurse Can we define tests that run nightly instead of on every PR/checking using the YAML of DOOM?

@halter73
Copy link
Member Author

I'm looking at signalr-daily-tests.yml, but I don't see the trigger.

@halter73
Copy link
Member Author

halter73 commented Jun 27, 2019

I see the comment about scheduled triggers, but I don't see any "schedules:" in any yaml files. Do I need an AzDo admin account?

https://github.com/aspnet/AspNetCore/blob/master/.azure/pipelines/signalr-daily-tests.yml#L1-L2

@analogrelay
Copy link
Contributor

Schedules in YAML didn't exist when we made the SignalR daily tests.

@halter73 halter73 self-assigned this Jul 17, 2019
@halter73 halter73 changed the title Run automated tests on debug builds or remove all Debug.Asserts Run automated tests on debug builds Jul 17, 2019
@halter73 halter73 removed the Working label Feb 25, 2020
@dougbu
Copy link
Member

dougbu commented Sep 14, 2020

Might be worthwhile but sounds more like a DoI issue

@dougbu
Copy link
Member

dougbu commented Nov 19, 2020

Adding @JunTaoLuo as an assignee. Suggest focusing on daily aspnetcore-ci and aspnetcore-helix-matrix runs that override the Configuration value just in that scheduled build.

@dougbu
Copy link
Member

dougbu commented Nov 19, 2020

Note obvious answer of adding a variable override to a schedules entry doesn't work: https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=azure-devops&tabs=schema%2Cparameter-schema#scheduled-trigger

@wtgodbe wtgodbe added affected-few This issue impacts only small number of customers severity-minor This label is used by an internal tool task labels Dec 3, 2020 — with ASP.NET Core Issue Ranking
@JunTaoLuo JunTaoLuo removed their assignment Jun 17, 2021
@halter73 halter73 removed their assignment Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-few This issue impacts only small number of customers area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework severity-minor This label is used by an internal tool task
Projects
None yet
Development

No branches or pull requests

8 participants
@analogrelay @halter73 @benaadams @JunTaoLuo @dougbu @BrennanConroy @wtgodbe and others