feat: module of utilities for integration tests

nginxinc / ngx-rust

Rust binding for NGINX

Apache License 2.0

713 stars 59 forks source link

feat: module of utilities for integration tests #74

Open JyJyJcr opened 3 months ago

JyJyJcr commented 3 months ago

Proposed changes

This PR adds test_util module, utilities for integration tests of Nginx modules. The utilities were moved from tests/log_test.rs, and fixed to use the bundled Nginx used in bindings, the install path of which is recorded in nginx-sys::metadata::NGINX_INSTALL_DIR.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

[x] I have written my commit messages in the Conventional Commits format.
[x] I have read the CONTRIBUTING doc
[x] I have added tests (when possible) that prove my fix is effective or that my feature works
[x] I have checked that all unit tests pass after adding my changes
[x] I have updated necessary documentation
[x] I have rebased my branch onto master
[x] I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

dekobon commented 3 months ago

Hi there, once again thank you for all of your contributions. I talked over this PR with some of the other project maintainers and there is a strong opinion that this functionality may be better in another project/library that supplements ngx-rust. Would it be possible to do that rather than integrate it into the core SDK?

JyJyJcr commented 3 months ago

Of course possible (and I agree to split it from the core), but before changing let me clarify which is better, creating a new GitHub repository, or just separating it into a new subdirectory in this repository, just like nginx-sys? Sidenote: we need to keep nginx-sys::metadata module to salvage build informations for integration tests in either case.

bavshin-f5 commented 3 months ago

Sidenote: we need to keep nginx-sys::metadata module to salvage build informations for integration tests in either case.

There's nginx-sys::NGX_PREFIX that already contains the installation path (NULL-terminated u8 slice, so you need to strip the last byte and use std::os::unix::ffi::OsStrExt).

It's also optional and could be omitted by the buildsystem if you specify an empty --prefix (we do that for the official Windows builds), but that's only a problem with non-vendored Nginx source tree.

JyJyJcr commented 2 months ago

There's nginx-sys::NGX_PREFIX that already contains the installation path (NULL-terminated u8 slice, so you need to strip the last byte and use std::os::unix::ffi::OsStrExt).

Thank you for important information. Now test_util directly uses constants in bindings.rs (not only NGX_PREFIX but also other configurations, such as NGX_CONF_PATH).