-
Notifications
You must be signed in to change notification settings - Fork 49
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
Login modifier reads from env #583
base: master
Are you sure you want to change the base?
Conversation
519da53
to
36ac96e
Compare
|
||
import com.cognifide.aet.job.api.exceptions.ParametersException; | ||
import com.googlecode.zohhak.api.TestWith; | ||
import com.googlecode.zohhak.api.runners.ZohhakRunner; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a completely separate conversation but, out of curiosity, have you considered migrating to JUnit 5? Something I could help with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, generally this is good idea to migrate to JUnit 5 - but there was never time for that.
This is definitely the thing you could help with and create a PR 😃
Map<String, String> params = new HashMap<>(); | ||
params.put(LOGIN_PAGE_PARAM, "whatever"); | ||
params.put(PASSWORD_PARAM, "${MY_SECRET_PASSWORD}"); | ||
environmentVariables.set("MY_SECRET_PASSWORD", "pass-form-env"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo?
environmentVariables.set("MY_SECRET_PASSWORD", "pass-form-env"); | |
environmentVariables.set("MY_SECRET_PASSWORD", "pass-from-env"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's typo I will fix that in IDE together with assertion. Thanks for spotting.
@@ -36,11 +37,11 @@ | |||
|
|||
private static final String DEFAULT_LOGIN_TOKEN = "login-token"; | |||
|
|||
private static final String LOGIN_PARAM = "login"; | |||
static final String LOGIN_PARAM = "login"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I'm reading this right but if this is literally going to be the name of the Environment variable to set, how about prefixing it with something more explicit?At the moment, it's quite generic and not immediately associated with AET, a kind of thing that might get users and administrators confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This constant is the name of a param that is passed via XML suite definition. The actual environment variable name is configured through that param, see the example:
<login login="${LOGIN_EXAMPLE_ADMIN_PANEL}"
password="${PASS_EXAMPLE_ADMIN_PANEL}"
login-page="/login.html"
login-input-selector="//input[@name='j_username']"
password-input-selector="//input[@name='j_password']"
submit-button-selector="//*[@type='submit']" />
36ac96e
to
f73bed6
Compare
Description
Login modifier can read
login
andpassword
from environment variables.Motivation and Context
Fixes #512
Types of changes
Checklist:
I hereby agree to the terms of the AET Contributor License Agreement.