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

Merge extra_init_by_lua_start and extra_init_by_lua directive in test cases #7990

Open
spacewander opened this issue Sep 26, 2022 Discussed in #7988 · 10 comments
Open

Merge extra_init_by_lua_start and extra_init_by_lua directive in test cases #7990

spacewander opened this issue Sep 26, 2022 Discussed in #7988 · 10 comments
Labels

Comments

@spacewander
Copy link
Member

Discussed in #7988

Originally posted by monkeyDluffy6017 September 26, 2022
Maybe we should merge extra_init_by_lua_start and extra_init_by_lua directives in test cases, they are just different on position.

@mikechengwei
Copy link

/assign

@ro4i7
Copy link

ro4i7 commented Mar 12, 2023

Hello @spacewander

if this issue is still open, please assign it to me,

The extra_init_by_lua_start and extra_init_by_lua directives are currently used separately in the test cases, here is an implementation to merge them into a single directive:

-- Before
ngx.config.debug = true

-- extra_init_by_lua_start
local mock_service = require("spec.plugins.proxy-cache.mock_service")
mock_service:start()

-- extra_init_by_lua
local cjson = require("cjson")
cjson.encode_empty_table_as_object(false)

-- After
ngx.config.debug = true

-- extra_init_by_lua
local mock_service = require("spec.plugins.proxy-cache.mock_service")
mock_service:start()

local cjson = require("cjson")
cjson.encode_empty_table_as_object(false)

This implementation moves the code from extra_init_by_lua_start to extra_init_by_lua, so that both directives are used together in a single block. This should have the same effect as before, but with a cleaner and more concise code.

@spacewander
Copy link
Member Author

Simply moving the code from extra_init_by_lua_start to extra_init_by_lua doesn't work.

@spacewander
Copy link
Member Author

Simply moving the code from extra_init_by_lua_start to extra_init_by_lua doesn't work.

We have tried it.

@ro4i7
Copy link

ro4i7 commented Mar 14, 2023

Simply moving the code from extra_init_by_lua_start to extra_init_by_lua doesn't work.

We have tried it.

Ok, thanks for updating. Exploring for another solutions.

@ro4i7
Copy link

ro4i7 commented Mar 14, 2023

Hello @spacewander

one probable solution may be, we can create a new directive called init_by_lua and use it in our test cases like this:

-- Before
ngx.config.debug = true

-- extra_init_by_lua_start
local mock_service = require("spec.plugins.proxy-cache.mock_service")
mock_service:start()

-- extra_init_by_lua
local cjson = require("cjson")
cjson.encode_empty_table_as_object(false)

-- After
local function init()
  local mock_service = require("spec.plugins.proxy-cache.mock_service")
  mock_service:start()

  local cjson = require("cjson")
  cjson.encode_empty_table_as_object(false)
end

ngx.config.debug = true
ngx.on_abort(init)

the code from extra_init_by_lua_start is moved into a function called init(), and extra_init_by_lua now calls this function using ngx.on_abort(). This should have the same effect as before.

@spacewander
Copy link
Member Author

You can try it.

@ro4i7
Copy link

ro4i7 commented Mar 16, 2023

You can try it.

So, I can create PR for this, please assign it to me?

@ro4i7
Copy link

ro4i7 commented Mar 17, 2023

You can try it.
@spacewander please check #9107

@spacewander
Copy link
Member Author

I take a look at it, it doesn't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment