Umegaya/test improvement

[umegaya]

this pull request aims to improve test environment and fix #59 bug.

test improvement

• fix broken tests
• prepare for docker image for running tests
  • also add task make ci to run tests on it
• use resty-cli as default lua runtime to run tests, because of lua-resty-http is now added to available.lua
  • for backward compatibility, able to choose lua runtime via argument to make command like make ci LUA=luajit
• add test instruction to README.md

PR #59 bug fix

util.slice is expected to behave like Array.slice, especially for last argument end, that is, returned array should not contain the element corresponding to the position of end. but actually was not at first implementation. because lua's for loop covers its 2nd argument.

as a result, first implementation does not pass test/modules/region_config.lua, because cn-north-1 was converted to cn-north-* by generateRegionPrefix, not cn-* as its expected to be.

[umegaya]

@nkzawa hi, I have opened PR to fix test. now it should pass at your environment with make ci. also, if you have a time, can you check my fix about your #59?

regards,

[nkzawa]

Thank you for improving tests. Using Docker for running tests seems awesome! The fix for slice also looks good to me.

But make ci MOCK=1 fails for me because api:region() returns false.

dynamodb:v[2012-08-10]:ListTables:error:./lua-aws/signers/v4.lua:60: invalid value (boolean) at index 2 in table for 'concat' @ stack traceback:
        ./lua-aws/api.lua:142: in function <./lua-aws/api.lua:141>
        [C]: in function 'concat'
        ./lua-aws/signers/v4.lua:60: in function 'credential_string'
        ./lua-aws/signers/v4.lua:153: in function 'authorization'
        ./lua-aws/signers/v4.lua:132: in function 'add_authorization'
        ./lua-aws/signers/v4.lua:15: in function 'sign'
        ./lua-aws/requests/base.lua:27: in function <./lua-aws/requests/base.lua:18>

[umegaya]

thank you for testing. I believe the crash caused by EC2_URL environment variable is not set. it should raise error like 'neither config.endpoint given nor EC2_URL environment set.' but current environment variable validity check overlooked the fault condition. I will fix this.

but I think the real problem is, now no one seems to use EC2_URL as region configuration (sorry, this project is about 5yrs old :<), so I will use AWS_DEFAULT_REGION to detect region.

do you think this helps current AWS api users?

[nkzawa]

👍 for AWS_DEFAULT_REGION

btw aws-sdk-js uses AWS_REGION instead of AWS_DEFAULT_REGION. Not sure which is appropriate tho. Additionally, I think it would be great if make ci MOCK=1 works without AWS_DEFAULT_REGION or any other environment variables set since the mock server would just work the same whatever the region is (I guess).

I tested make ci MOCK=1 AWS_DEFAULT_REGION=us-west-1, but it still fails with a different error:

dynamodb:v[2012-08-10]:ListTables:error:./lua-aws/requests/base.lua:32: attempt to compare string with number @ stack traceback:
        ./lua-aws/api.lua:145: in function '__le'
        ./lua-aws/requests/base.lua:32: in function <./lua-aws/requests/base.lua:18>
        [C]: in function 'xpcall'
        ./lua-aws/api.lua:141: in function 'listTables'

resp.status is a string "connection refused" for me 🤔 https://github.com/umegaya/lua-aws/blob/master/lua-aws/requests/base.lua#L31

[umegaya]

I cannot see AWS_REGION in official AWS CLI environment variable document, so wanna use AWS_DEFAULT_REGION for now.

about error, it happens repeatedly? if so, I think sleep 2 (https://github.com/umegaya/lua-aws/blob/master/test/tools/run.sh#L32) may not enough for your environment. if problem solved when you set this longer, I will figure out the way to know when moto server spinned up correctly.

[nkzawa]

I agreed about the environment variable.

sleep 4 fixes the problem, thank you. sleep 2 always fails on my environment. It'd be nice if we can detect the server ready by stdout.