ngx.process.master_pid

[chronolaw]

Here is the new API ngx.process.master_pid, which needs ffi C function ngx_http_lua_ffi_master_pid in ngx_lua.

I also write a test case in t/process-master-pid.t, But I am not familar with Perl and have not found the detail API documents of test::nginx, so I just copied worker.t then modified on it.

That test case may has error, please help me to correct it. Thanks very much.

[membphis]

you need to change the travis config, use your own branch to test.

https://github.com/openresty/lua-resty-core/blob/master/.travis.yml#L62

[agentzh]

This test is too loose. We could do better here by comparing the number returned by master_pid() against the real master pid saved in the nginx pid file.

[agentzh]

This is definitely wrong. You missed a pair of parentheses after process.master_pid. Have you actually run this test yourself?

This is another reason why you should make your PR pass the travis ci checks as @membphis suggested ;)

[chronolaw]

Sorry, I'm not familar with travis ci, I will try to correct it ,thanks.

[chronolaw]

And here is not an error ,because pid will be called in then next line ：
local v = pid()

[agentzh]

@chronolaw I see, but the pid function name is indeed confusing. How about using the variable name get_pid here? So that there could be no ambiguity here.

[chronolaw]

I have run test in my own nginx 1.13.8+ngx_lua，and I will try to config travis ci after soon.

[chronolaw]

I have tried travis-ci build, but I got an error.

Then reason is that the process.master_pid needs nginx 1.13.8, but now OpenResty build will fail on it(no proper patch for it).

And in nginx 1.13.6, this API will return nil for no ngx_parent in nginx core.

Would you give me some advise to write the test case?

[chronolaw]

I created a new git branch to fit nginx 1.13.6, now process.master_pid passed ,but got another error

#   Failed test 'TEST 1: ngx.process.master_pid - pattern "(?^:\[TRACE   \d+ content_by_lua\(nginx\.conf:\d+\):4 loop\])" should match a line in error.log (req 0)'
#   at /home/travis/build/chronolaw/lua-resty-core/test-nginx/lib/Test/Nginx/Socket.pm line 1206.
#   Failed test 'TEST 1: ngx.process.master_pid - pattern "(?^:\[TRACE   \d+ content_by_lua\(nginx\.conf:\d+\):4 loop\])" should match a line in error.log (req 1)'

So could you tell my how to write the right error_log eval in xxx.t?

[chronolaw]

After some modify, my own test has passed, please see:https://travis-ci.org/chronolaw/lua-resty-core/jobs/324018825

Changes:
use chronolaw/lua-nginx-module.git and branch test_ffi_master_pid. In this branch code is

#if (nginx_version >= 1013008)
    return (int) ngx_parent;
#else
    return (int) ((ngx_process == NGX_PROCESS_SINGLE) ? ngx_pid : getppid());
    //return -1;
#endif

[agentzh]

@chronolaw You can skip the tests for nginx cores older than 1.13.8 with the --- skip_nginx section. You can see such examples in this library's existing test suite.

[agentzh]

@chronolaw And you should point to your own fork of lua-nginx-module in .travis.yml in your branch, see others' PRs for such examples, like this:

#89

[agentzh]

We should not put extra blank lines at the end of the .t test files. The tool reindex can automatically format your .t files:

https://github.com/openresty/openresty-devel-utils/blob/master/reindex

[agentzh]

For the sake of debugging test failures, I think we should also output the actual master pid in the .pid file in case of comparison failures.

[agentzh]

@chronolaw I see, but the pid function name is indeed confusing. How about using the variable name get_pid here? So that there could be no ambiguity here.

[agentzh]

You need to add the following line here to enable nginx master process:

master_on();

The test scaffold does not turn on nginx master process by default. Without this line, this test file fails on my side:

t/process-master-pid.t .. 1/12
#   Failed test 'TEST 1: ngx.process.master_pid - response_body_like - response is expected (false 69514)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1601.
#                   'false
# 69514
# '
#     doesn't match '(?^s:^true
# \d+$)'

#   Failed test 'TEST 1: ngx.process.master_pid - response_body_like - response is expected (false 69514)'
#   at /home/agentzh/git/lua-resty-core/../test-nginx/lib/Test/Nginx/Socket.pm line 1601.
#                   'false
# 69514
# '
#     doesn't match '(?^s:^true
# \d+$)'
# Looks like you failed 2 tests of 12.
t/process-master-pid.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/12 subtests

Test Summary Report
-------------------
t/process-master-pid.t (Wstat: 512 Tests: 12 Failed: 2)
  Failed tests:  2, 8
  Non-zero exit status: 2
Files=1, Tests=12,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.09 cusr  0.01 csys =  0.11 CPU)
Result: FAIL

[agentzh]

Style: needs spaces around binary operators like ...

Also, io.open() may return an error, better wrap it with an assert() call.

[agentzh]

Merged by fixing the aforementioned issues myself. Thanks!