-
Notifications
You must be signed in to change notification settings - Fork 2
Optimisations for windows #4
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
Conversation
|
|
| "Windows|11|Edge", | ||
| "Windows|11|Chrome", | ||
| "Windows|8|Chrome", | ||
| #"OS X|Monterey|Safari", |
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.
remove commented code lines.
| #"OS X|Monterey|Safari", | ||
| "OS X|Monterey|Chrome", | ||
| "OS X|Ventura|Chrome", | ||
| #"OS X|Big Sur|Safari", |
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.
remove commented code lines.
| $second = [int]$parts[1] | ||
| if ($first -eq 10) { return $true } | ||
| if ($first -eq 192 -and $second -eq 168) { return $true } | ||
| if ($first -eq 172 -and $second -ge 16 -and $second -le 31) { return $true } |
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.
good logic.
| try { | ||
| # Update Base URL | ||
| $files = Get-ChildItem -Path $TARGET -Recurse -Filter *.* -File | Where-Object { $_.Extension -match '\.(java|xml|properties)$' } | ||
| foreach ($file in $files) { | ||
| $content = Get-Content $file.FullName -Raw -ErrorAction SilentlyContinue | ||
| if ($content -and $content -match "https://www\.bstackdemo\.com") { | ||
| $content = $content -replace "https://www\.bstackdemo\.com", $CX_TEST_URL | ||
| Set-ContentNoBom -Path $file.FullName -Value $content | ||
| Log-Line "🌐 Updated base URL in $($file.Name)" $GLOBAL_LOG | ||
| } |
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.
@himanshu-02 - would we need this if we set the CX_TEST_URL in an ENV and use the repos where URL is fetched using the ENV?
| if (Test-Path $testFileFull) { | ||
| $c = [System.IO.File]::ReadAllText($testFileFull) | ||
| $c = $c.Replace("https://bstackdemo.com", $CX_TEST_URL) | ||
| Set-ContentNoBom -Path $testFileFull -Value $c | ||
| Log-Line "🌐 Updated base URL in tests/bstack-sample-test.py to: $CX_TEST_URL" $GLOBAL_LOG | ||
| } |
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.
@himanshu-02 - would we need this if we set the CX_TEST_URL in an ENV and use the repos where URL is fetched using the ENV?
| $LOCAL_FAILURE = $false | ||
| $SETUP_FAILURE = $false | ||
|
|
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.
These are not being used across the board. Let's remove for now, from a consistency perspective.
| } | ||
| Log-Line "========================================" $GLOBAL_LOG | ||
| Log-Line "🎉 All requested tests have been executed!" $GLOBAL_LOG | ||
| Log-Line "🔗 View results: https://automate.browserstack.com/ (Web) | https://app-automate.browserstack.com/ (Mobile)" $GLOBAL_LOG |
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 should be only one endpoint:
View results: https://automation.browserstack.com/
|
thanks for the review @samirans89, will incorporate these changes. |
No description provided.