godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.17k stars 98 forks source link

Add unit testing for request-response lifecycle in networking subsystems #9581

Open GrammAcc opened 7 months ago

GrammAcc commented 7 months ago

Describe the project you are working on

Godot engine development - Networking.

Describe the problem or limitation you are having in your project

I'm currently working on fixing godotengine/godot#90160, and I noticed that we don't currently have a way to test actual HTTP requests with the engine's unit tests. Having a test server of some kind to test the request-response cycle is necessary for making changes to networking APIs without worrying about regressions, and this is something that should be a part of Godot's standard test suite.

I have discussed this with networking contributors and maintainers in the contributors chat, and the general consensus is that any solution needs to satisfy the following requirements:

  1. It needs to be an actual HTTP server, not a library or mock server.
  2. It cannot inconvenience contributors to other areas of the engine.
  3. It needs to be a server that is used in production and supports different versions of the HTTP spec.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Adding a test server or some kind of automatic setup of a test server to the unit testing environment allows us to write unit tests that make a request to a particular URL and assert a certain response, which helps prevent the worst regressions when working on networking code.

Most developers would probably consider these to be integration tests, not unit tests, but I've never found value in distinguishing between the two. I think a better distinction is fast vs slow. Currently, we don't have the ability to validate that a change doesn't break the networking APIs, so some kind of solution is needed here.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Based on the discussion in the contributors chat, I currently have three possible solutions for this:

Option 1 - Dockerized Server

I can containerize an Apache or Nginx server in order to simplify deployment between local development and CI. The advantage of this approach is:

The disadvantages are

  1. The added complexity of Docker, which is not nearly as easy to maintain as the hype would have you believe.
  2. Developers would still have to set up Docker or Podman on their machines to run the server, so it isn't automatic setup.

Option 2 - Automatic Native Setup

I can add some bash scripts to curl a release binary and verify checksums in order to run a server natively on the dev machine without needing the user to install or configure anything. This would be easiest with Caddy. Not sure if this is even possible with Apache or Nginx.

The advantages of this approach are:

  1. It is automagical from the developer's perspective.
  2. It doesn't require a lot of extra infrastructure on our part, only a server config and some bash.

The disadvantages are:

  1. It has a much higher chance of running into compat issues that could cause misleading test failures.
  2. It doesn't work on Windows, so even if we did this, we'd still need Docker to run it cross-platform.
  3. It locks us into a specific server since the process of deploying the server would likely be very different for each one, so the switching cost is very high.

Option 3 - Shared Config

Instead of managing a test server on behalf of the user, I can add config files for different servers (probably Apache, Nginx, and Caddy at this point), and then the user can simply start the server locally with the provided config.

This has these advantages:

  1. Much lower maintenance burden and switching cost. Changing/adding a server is as simple as porting a config file.
  2. We can run the same unit tests against multiple production servers which better simulates real-world usage and could help catch really hard-to-find compat bugs between different servers/protocols.
  3. We can still use Docker to spin up servers in CI, so this also works across platforms and environments.

This has these disadvantages:

  1. We have to maintain multiple server configs instead of only one.
  2. Networking contributors still need to spin up their own servers locally to test their code.

Summary

All three of the above solutions will require some kind of flag to disable live network tests when contributing to other parts of the engine. Even if we can make the process completely automatic, it will slow things down for developers that aren't changing network code. Fales in the contributor's chat suggested that we may be able to change the --test flag so that it doesn't run the live network tests by default and they need to be opted into with a separate flag or filter. I think this is the best approach.

Personally, I prefer option 3 because it is very easy to spin up a server on a local machine if you have a valid config, but it is extremely difficult to spin up a valid server on someone else's machine. I also like how it allows us to test against multiple servers in multiple environments.

I don't think the maintenance burden of multiple server configs will be that bad. Most of the config will rarely change. Adding a test case will involve adding a location/route to each server config with the expected response, but this is a very easy and small change usually. I don't think most networking contributors will need to change the actual configuration of the server itself.

Although option 3 is the best I could think of, there may be better solutions out there, so if anyone has any ideas for how to make this easier, please speak up. Thanks!

If this enhancement will not be used often, can it be worked around with a few lines of script?

No, it is part of the engine development workflow.

Is there a reason why this should be core and not an add-on in the asset library?

It's part of the build/development workflow, so I don't think it makes sense for this to be an addon, even if it could be.

m4rr5 commented 7 months ago

While reading the proposal I expected another option:

Option 4 - Add an HTTP server to Godot

This one can either be implemented natively or in GDscript, which is not just useful for testing, but also for example to host a REST API that can be used in a multiplayer setup.

GrammAcc commented 7 months ago

While reading the proposal I expected another option:

Option 4 - Add an HTTP server to Godot

This one can either be implemented natively or in GDscript, which is not just useful for testing, but also for example to host a REST API that can be used in a multiplayer setup.

This was my first thought as well. I was planning on adding a simple server to be used only in the test suite since it would allow for really concise syntax for declaring routes and responses inside unit tests. This was discussed in the contributor's chat, and @faless (the current networking maintainer) confirmed that it would be too much of a maintenance burden to ensure it actually produces standards-compliant responses for regression testing.

Adding an HTTP server for use in projects is an interesting idea, but that seems like it's outside the scope of this proposal, which focuses on internal testing. The server would need to be tested thoroughly itself and probably wouldn't be suitable to be used to drive tests for other modules.

Maybe other maintainers can weigh in on the pros and cons with a custom test server though.

jonbonazza commented 7 months ago

option 4 is pretty much a non starter for the following reasons:

  1. we don’t want to write and maintain our own http server. thats sounds terrible. i already dont like that we have to maintain our own http client, but i digress.
  2. we don’t want to bloat the application with an http server library that will only be used for testing.
  3. i dont think we have a great way currently to only include libraries for test and not include them for builds.
m4rr5 commented 7 months ago

Apart from the argument if we "want or don't want" to include an HTTP server and ignoring the fact that HTTP is probably one of the most important protocols out there (also for games) I am a bit puzzled by statements like "we don't want to write and maintain our own HTTP server", as if there are no existing ones out there that can easily be embedded. I agree writing an HTTP server from scratch would not make sense, but there are many existing ones out there that are designed to be embedded. In terms of it being part of the "core" of Godot or a GDExtension, I think that's a fair debate and I could easily see this being shipped as an extension (or, if part of the core, a module you can turn off).

GrammAcc commented 7 months ago

I get where you're coming from, but embedding a third-party server is not trivial. Any dependency is an additional maintenance burden in and of itself, and something like an HTTP server adds a large attack vector to the engine that we need to keep track of. Even moreso if someone else wrote and is maintaining the code. If the maintainers of the third-party server are not responsive enough to CVEs, then we would have to do something about it to protect our users, most likely submitting an upstream patch, but it's possible the upstream maintainers wouldn't accept it, so we'd have to patch the project locally/maintain a fork until such issues get resolved upstream.

It's counterintuitive, but sometimes it's actually less maintenance to just write something in-house instead of adding a third-party dependency. In the case of a server, probably not, but that's because the maintenance burden is so massive regardless of how it's integrated.

Also, for the Godot engine, there's the issue of license compatibility. IIUC, we can't include any dependencies that are licensed under the GPL, so that significantly limits our options when selecting libraries.

In any case, including an HTTP server in the engine for use in actual projects is a separate proposal as this proposal is only about testing the engine's networking APIs. :)

Calinou commented 7 months ago

This one can either be implemented natively or in GDscript, which is not just useful for testing, but also for example to host a REST API that can be used in a multiplayer setup.

The editor already features an HTTP server (with TLS support + self-signed certificate generation). This web server is used to host the exported web project when using one-click deploy.

However, it's not exposed to the scripting API and is only compiled in editor builds. This is intentionally done to decrease the maintenance burden and attack surface. Some projects of mine would benefit from an HTTP server, but a GDScript implementation is fast and full-featured enough for my needs.

Anutrix commented 6 months ago

This one can either be implemented natively or in GDscript, which is not just useful for testing, but also for example to host a REST API that can be used in a multiplayer setup.

The editor already features an HTTP server (with TLS support + self-signed certificate generation). This web server is used to host the exported web project when using one-click deploy.

However, it's not exposed to the scripting API and is only compiled in editor builds. This is intentionally done to decrease the maintenance burden and attack surface. Some projects of mine would benefit from an HTTP server, but a GDScript implementation is fast and full-featured enough for my needs.

For my current Godot project, I need to send some large local files between Godot applications running in 2 devices on same network. Super grateful that HTTPRequest exists in Godot. Now, similarly I am looking for a cross-platform Godot HTTPS server for multiple reasons including above.

I found that my options include either https://github.com/deep-entertainment/godottpd or the one-click deploy's editor-only HTTP server I found in the source. godottpd doesn't seem to support TLS yet so I need to create an server(with TLS) addon/project myself or exposing one-click deploy's server to GDScript.

I would prefer latter but am fine with GDScript implementation as well if it's full-featured and secure. I wonder if there is a GDScript implementation already released by someone(other than godottpd).

GrammAcc commented 6 months ago

For my current Godot project, I need to send some large local files between Godot applications running in 2 devices on same network. Super grateful that HTTPRequest exists in Godot. Now, similarly I am looking for a cross-platform Godot HTTPS server for multiple reasons including above.

I have updated the title of this proposal since I think the original was misleading.

This proposal is about unit testing Godot's networking APIs for engine contributors, which doesn't necessarily mean standing up a server in the test suite (though that is the most likely solution). Please open a separate proposal to discuss adding a production server for use in Godot projects as that is outside the scope of this proposal.

Thanks!