Skip to content

test: Refactor tests to use env instead of opt vars #672

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Feb 13, 2022
Merged

test: Refactor tests to use env instead of opt vars #672

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a0997e2
fix: Convert deprecated CLI arguments to env vars in tests
Feb 11, 2022
f14e168
Fix argument parsing
Feb 11, 2022
661c127
Prettify test.js
Feb 11, 2022
5aeee22
Parse sls commands to input deprecated arguments as options
Feb 12, 2022
64d37e3
Enable passing options other than env
Feb 12, 2022
fe52dc1
Fix variable
Feb 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,25 @@ const initialWorkingDir = process.cwd();
const mkCommand =
(cmd) =>
(args, options = {}) => {
let envVars = { SLS_DEBUG: 't' };
if (cmd == 'sls') {
// convert deprecated CLI arguments to env vars
const argsCopy = [...args];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is a working solution, I think it would be better to explicitly pass these options as env to sls created with mkCommand utility. What do you think? For example, invocation could look like this:

sls(['package'], {env: {DOCKERIZE_PIP: true, SLIM: true}});

instead of

sls(['--dockerizePip=true', '--slim=true', 'package']);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree, but I'd need to spend time on manually rewriting each function... ;/
Maybe I'll come up with a script to parse these lines for me if you insist on this - if you don't, then I think it's gonna be good to merge after I push the lint correction.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @martinezpl - I think we will need to address it anyway - if you won't have time to address it, I can give it a try, I'd rather not merge in code that needs to be changed soon. Let me know 💯

Copy link
Contributor Author

@martinezpl martinezpl Feb 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, turned out to be a pretty quick task with a bit of regex.

-edit
I've run into some problems after implementing the convention you suggested - the sls command threw NOENT errors, that's why the code in my commit differs.

However now I realised I just cut out the way to pass options different than 'env'. Seems like it hasn't been used so far, but it's probably better to have it. Let me correct.

-edit2
Done!

args = [];
for (let i = 0; i < argsCopy.length; i++) {
if (argsCopy[i].includes('--')) {
envVars[argsCopy[i].slice(2).split('=')[0]] = argsCopy[i].split('=')[1];
} else {
args.push(argsCopy[i]);
}
}
}
const { error, stdout, stderr, status } = crossSpawn.sync(
cmd,
args,
Object.assign(
{
env: Object.assign({}, process.env, { SLS_DEBUG: 't' }),
env: Object.assign({}, process.env, envVars),
},
options
)
Expand All @@ -45,6 +58,7 @@ const mkCommand =
}
return stdout && stdout.toString().trim();
};

const sls = mkCommand('sls');
const git = mkCommand('git');
const npm = mkCommand('npm');
Expand Down
22 changes: 11 additions & 11 deletions tests/base/serverless.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,22 @@ service: sls-py-req-test

provider:
name: aws
runtime: ${opt:runtime, 'python3.6'}
runtime: ${env:runtime, 'python3.6'}

plugins:
- serverless-python-requirements
custom:
pythonRequirements:
zip: ${opt:zip, self:custom.defaults.zip}
dockerizePip: ${opt:dockerizePip, self:custom.defaults.dockerizePip}
slim: ${opt:slim, self:custom.defaults.slim}
zip: ${env:zip, self:custom.defaults.zip}
dockerizePip: ${env:dockerizePip, self:custom.defaults.dockerizePip}
slim: ${env:slim, self:custom.defaults.slim}
slimPatterns: ${file(./slimPatterns.yml):slimPatterns, self:custom.defaults.slimPatterns}
slimPatternsAppendDefaults: ${opt:slimPatternsAppendDefaults, self:custom.defaults.slimPatternsAppendDefaults}
vendor: ${opt:vendor, ''}
fileName: ${opt:fileName, 'requirements.txt'}
useStaticCache: ${opt:useStaticCache, self:custom.defaults.useStaticCache}
useDownloadCache: ${opt:useDownloadCache, self:custom.defaults.useDownloadCache}
cacheLocation: ${opt:cacheLocation, ''}
slimPatternsAppendDefaults: ${env:slimPatternsAppendDefaults, self:custom.defaults.slimPatternsAppendDefaults}
vendor: ${env:vendor, ''}
fileName: ${env:fileName, 'requirements.txt'}
useStaticCache: ${env:useStaticCache, self:custom.defaults.useStaticCache}
useDownloadCache: ${env:useDownloadCache, self:custom.defaults.useDownloadCache}
cacheLocation: ${env:cacheLocation, ''}
defaults:
slim: false
slimPatterns: false
Expand All @@ -29,7 +29,7 @@ custom:
useDownloadCache: true

package:
individually: ${opt:individually, self:custom.defaults.individually}
individually: ${env:individually, self:custom.defaults.individually}
patterns:
- '!**/*'
- 'handler.py'
Expand Down
2 changes: 1 addition & 1 deletion tests/individually/serverless.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ package:
- '!node_modules/**'
custom:
pythonRequirements:
dockerizePip: ${opt:dockerizePip, self:custom.defaults.dockerizePip}
dockerizePip: ${env:dockerizePip, self:custom.defaults.dockerizePip}
defaults:
dockerizePip: false

Expand Down
8 changes: 4 additions & 4 deletions tests/pipenv/serverless.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ plugins:
- serverless-python-requirements
custom:
pythonRequirements:
zip: ${opt:zip, self:custom.defaults.zip}
slim: ${opt:slim, self:custom.defaults.slim}
zip: ${env:zip, self:custom.defaults.zip}
slim: ${env:slim, self:custom.defaults.slim}
slimPatterns: ${file(./slimPatterns.yml):slimPatterns, self:custom.defaults.slimPatterns}
slimPatternsAppendDefaults: ${opt:slimPatternsAppendDefaults, self:custom.defaults.slimPatternsAppendDefaults}
dockerizePip: ${opt:dockerizePip, self:custom.defaults.dockerizePip}
slimPatternsAppendDefaults: ${env:slimPatternsAppendDefaults, self:custom.defaults.slimPatternsAppendDefaults}
dockerizePip: ${env:dockerizePip, self:custom.defaults.dockerizePip}
defaults:
zip: false
slimPatterns: false
Expand Down
8 changes: 4 additions & 4 deletions tests/poetry/serverless.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ plugins:
- serverless-python-requirements
custom:
pythonRequirements:
zip: ${opt:zip, self:custom.defaults.zip}
slim: ${opt:slim, self:custom.defaults.slim}
zip: ${env:zip, self:custom.defaults.zip}
slim: ${env:slim, self:custom.defaults.slim}
slimPatterns: ${file(./slimPatterns.yml):slimPatterns, self:custom.defaults.slimPatterns}
slimPatternsAppendDefaults: ${opt:slimPatternsAppendDefaults, self:custom.defaults.slimPatternsAppendDefaults}
dockerizePip: ${opt:dockerizePip, self:custom.defaults.dockerizePip}
slimPatternsAppendDefaults: ${env:slimPatternsAppendDefaults, self:custom.defaults.slimPatternsAppendDefaults}
dockerizePip: ${env:dockerizePip, self:custom.defaults.dockerizePip}
defaults:
zip: false
slimPatterns: false
Expand Down