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

test(selenium): read config from CONNECTOR_AUTH_FILE_PATH environment variable and fix bugs in UI tests #1225

Merged
merged 36 commits into from
Jun 5, 2023

Conversation

jagan-jaya
Copy link
Contributor

@jagan-jaya jagan-jaya commented May 22, 2023

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

Closes #924

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

To support automatic api_key generation by accepting the configs from sample_auth file

How did you test it?

Screen Shot 2023-05-26 at 9 06 21 PM Screen Shot 2023-05-22 at 10 18 54 AM

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed submitted code
  • I added unit tests for my changes where possible

@jagan-jaya jagan-jaya added the A-connector-integration Area: Connector integration label May 22, 2023
@jagan-jaya jagan-jaya self-assigned this May 22, 2023
@jagan-jaya jagan-jaya requested review from a team as code owners May 22, 2023 05:10
ArjunKarthik
ArjunKarthik previously approved these changes May 22, 2023
Comment on lines 42 to 53
pub hs_base_url: Option<String>,
pub hs_api_key: Option<String>,
pub hs_test_browser: Option<String>,
pub chrome_profile_path: Option<String>,
pub firefox_profile_path: Option<String>,
pub pypl_email: Option<String>,
pub pypl_pass: Option<String>,
pub gmail_email: Option<String>,
pub gmail_pass: Option<String>,
pub configs_url: Option<String>,
pub stripe_pub_key: Option<String>,
pub testcases_path: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer moving all these to a struct instead, since it's quite easy to make mistakes when writing the TOML files for the current configuration structure.

@SanchithHegde SanchithHegde added the S-waiting-on-author Status: This PR is incomplete or needs to address review comments label May 22, 2023
@SanchithHegde SanchithHegde changed the title fix(tests): Read config from sample_auth file and bug fixes in ui tests test(selenium): read config from CONNECTOR_AUTH_FILE_PATH environment variable and fix bugs in UI tests May 22, 2023
@@ -489,5 +550,5 @@ pub fn handle_test_error(
}

pub fn get_env(name: &str) -> String {
env::var(name).unwrap_or_else(|_| panic!("{name} not present")) //Issue: #924
env::var(name).unwrap_or_else(|_| panic!("{name} not present"))
Copy link
Member

Choose a reason for hiding this comment

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

Add Issue id back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Configs from auth.toml takes priority over env variables, if you want to override configs you can use this, this is the not the primary setting

@SanchithHegde SanchithHegde added the S-needs-conflict-resolution Status: This PR needs conflicts to be resolved by the author label May 24, 2023
@AkshayaFoiger AkshayaFoiger requested review from a team and ashokkjag as code owners May 26, 2023 06:10
@jagan-jaya jagan-jaya removed the S-needs-conflict-resolution Status: This PR needs conflicts to be resolved by the author label May 26, 2023
@jagan-jaya jagan-jaya added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels May 30, 2023
@jagan-jaya jagan-jaya added S-ready-for-merge and removed S-waiting-on-review Status: This PR has been implemented and needs to be reviewed labels Jun 5, 2023
@jarnura jarnura added this pull request to the merge queue Jun 5, 2023
Merged via the queue into main with commit d9a16ed Jun 5, 2023
6 checks passed
@SanchithHegde SanchithHegde deleted the ui-tests-v2 branch June 5, 2023 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-integration Area: Connector integration
Projects
No open projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

[FEATURE] move env vars in selenium test to toml config file
5 participants