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

Support for collecting application coverage from Asp.Net Web Application (not UT) #1079

Closed
pkrukp opened this issue Feb 2, 2021 · 12 comments
Labels
duplicate This issue or pull request already exists

Comments

@pkrukp
Copy link

pkrukp commented Feb 2, 2021

Hi,

would be great if we could simply collect application coverage from a running Web Application. For example user starts web application and interacts normally with it to collect coverage. Currently this is not possible due to inability to gracefully shut down web application.

Details

I tried collecting it using example web application but the problem seems to be with closing the web application. Normally start it with "dotnet run" and close with "Ctrl+C" and it shuts down gracefully:

$ dotnet run
[...]
info: Microsoft.Hosting.Lifetime[0]
      Now listening on: http://localhost:5000
[Ctrl+C pressed]
info: Microsoft.Hosting.Lifetime[0]
      Application is shutting down...

But when this is run under coverlet.exe, it seems coverlet.exe is eating the Ctrl+C, does not pass it to the application, just exists:

$ coverlet bin/Debug/netcoreapp3.0/ -t  dotnet -a run
[...]
info: Microsoft.Hosting.Lifetime[0]
      Now listening on: http://localhost:5000
[Ctrl+C pressed]
[coverlet.exe exits, dotnet is still running in background]

I found #781 (comment) where you describe that collecting coverage from Web Application is generally possible but the problem is graceful termination of web application. Suggested workaround (modifying application source code) is not really a viable/user friendly solution.

@MarcoRossignoli
Copy link
Collaborator

For the moment we have only this way https://github.com/coverlet-coverage/coverlet/blob/master/Documentation/GlobalTool.md#code-coverage-for-integration-tests-and-end-to-end-tests

To allow "kill" we need to change the core of coverlet and move from "in-process" collection(array in memory) to "out of process" collection(shared memory or something like that).

There is an open issue for that #808

@MarcoRossignoli MarcoRossignoli added the duplicate This issue or pull request already exists label Feb 6, 2021
@MarcoRossignoli
Copy link
Collaborator

Close as duplicate

@pkrukp
Copy link
Author

pkrukp commented Feb 22, 2021

Sorry for responding late

To allow "kill" we need to change the core of coverlet and move from "in-process" collection(array in memory) to "out of process" collection(shared memory or something like that).

There are alternatives possible, for example add "instrument"/"un-instrument" mode to the coverlet.console.

As I understand the coverlet.console could instrument assemblies and then exit. Web Application would be run by user, not "wrapped" by coverlet.console. After Web Application exits, coverlet would be run again to remove instrumentation and report results.

That would "fix" also the other problem with not passing input to the wrapped application.

I guess moving to out of process collection is very hard compared to adding instrument/un-instrument modes.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 23, 2021

After Web Application exits, coverlet would be run again to remove instrumentation and report results.

It won't work...it's not possible subscribe to a "kill" we have the same problem as gracefully close.
The main problem of this approach is that atm coverlet save hits "in memory" process of test and we save on process exits event, so every kill/non gracefully close won't allow hits to be persisted on disk for late elaboration.

I mean After Web Application exits... can exit for a kill and we cannot do nothing FWIK. With coverlet wrapping test process we have the process exit event...but again the hits inside test process memory won't be persisted without the process exit callback of test process and in case of "kill" that callback is not called.

@pkrukp
Copy link
Author

pkrukp commented Feb 24, 2021

The point is that, as far as I know, dotnet web application server recognizes "Ctrl+C" and shutdowns gracefully. It prints the "Application is shutting down..." and takes some time to shutdown. I assume process exit handlers are called and coverage collection would work.

coverlet.console seems to kill processes - there is no "Application is shutting down..." printed.

(I assume instrumentation process adds code to collect and dump coverage to disk - it does not need coverage.console to be running during coverage collection or dumping)

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 24, 2021

Coverlet tool start process and wait for exit...does nothing on child process https://github.com/coverlet-coverage/coverlet/blob/master/src/coverlet.console/Program.cs#L129

coverlet.console seems to kill processes - there is no "Application is shutting down..." printed.

I think it's an output redirection issue in that case...because what you see is the coverlet console ui.

@pkrukp
Copy link
Author

pkrukp commented Feb 24, 2021

But I can see other output lines from the dotnet server when it's being run under coverlet (see initial comment with quoted output)... so it does not look like output redirection issue?

Since coverlet.console does not kill process, then I think the whole coverlet.console is killed when I press Ctrl+C.

I'm testing it on windows in the new "Windows Terminal" with "Git Bash" running. I guess after I press Ctrl+C in terminal, something either kills coverlet.console, or sends a termination signal to it which causes it to terminate.

(I just noticed my initial comment said "coverlet.exe exist" - it should be "coverlet.exe exits"... fixed it)

@MarcoRossignoli
Copy link
Collaborator

Try to verify if underneath process is exited using tool like process explorer...when child process exits coverlet should generate reports...if it doesn't to id it means that coverlet is waiting process exits.

@pkrukp
Copy link
Author

pkrukp commented Feb 24, 2021

As I wrote in initial comment:
[coverlet.exe exits, dotnet is still running in background]

@MarcoRossignoli
Copy link
Collaborator

Oh sorry you're right, for the moment we don't have better solution, I mean the only way to let it run is find a way to shutdown the child process gracefully(and fire process exits) with some custom code/workflow(maybe some call to host process at the end of test session that fires a gracefully shutdown of kestrel and close the host), Ctrl+C doesn't work, you're killing coverlet app in fact as you said.

Another solution could be add some custom internal code in a helper and every x seconds force the flush of code like here https://github.com/coverlet-coverage/coverlet/blob/master/src/coverlet.collector/InProcDataCollection/CoverletInProcDataCollector.cs#L66 anyway you need to find a way to "kill/shutdown" child process to let coverlet to generate the report.

I agree with you this feat is not complete/user friendly yet, it was a first attempt to give something on that side, as you can see the issue you linked is open again.

@pkrukp
Copy link
Author

pkrukp commented Feb 26, 2021

Another solution could be add some custom internal code in a helper and every x seconds force the flush of code

Yes, I agree that's a good idea in general. Not ideal in this case (won't capture coverage from code executed on shutdown), but useful in other use cases - for example when the application does not have a way to exit at all.

What do you think about the other proposition - adding "-instrument" and "-uninstrument"? It would provide additional flexibility and would be fully working workaround for this case (+ workaround for #1002).

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 26, 2021

What do you think about the other proposition - adding "-instrument" and "-uninstrument"? It would provide additional flexibility and would be fully working workaround for this case (+ workaround for #1002).

Could be an idea, there are some things to take into account(atm these comes to my mind)

  • Add 2 new switch -instrumentOnly -generateInstrumentOnlyReports (the target should be the fixed var in the workflow)
  • Save parameters into instrumentation artifacts(I'm working on it for another issue master...MarcoRossignoli:deterministicreport)
  • understand how to deal with multiple -instrument(check if already instrumented, cleanup old instrumentation artifacts, found the correct place where to put this stuff)
  • anyway the "kill" issue still remains

@pkruk2 can you open a new issue?I'll tag as new feature. If you want you can help with the implemetation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants