Skip to content

Use HTTPS in test.com test URL #8338

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

Draft
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Draft

Use HTTPS in test.com test URL #8338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6199031
DRY test site URL value in `AuthenticationManagerTests.swift`
mokagio Dec 7, 2022
dcb836d
Remove an unnecessary `siteURL` re-declaration in a test
mokagio Dec 7, 2022
56cb936
Use HTTPS instead of HTTP in `test.com` URL in unit tests
mokagio Dec 7, 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
2 changes: 1 addition & 1 deletion Networking/NetworkingTests/Remote/JetpackConnectionRemoteTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import XCTest

final class JetpackConnectionRemoteTests: XCTestCase {

private let siteURL = "http://test.com"
private let siteURL = "https://test.com"

/// Dummy Network Wrapper
///
Expand Down
4 changes: 2 additions & 2 deletions Networking/NetworkingTests/Requests/WordPressOrgRequestTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import XCTest

final class WordPressOrgRequestTests: XCTestCase {

private let baseURL = "http://test.com"
private let baseURL = "https://test.com"
private let path = "/test/request"

func test_request_url_is_correct() throws {
Expand All @@ -14,7 +14,7 @@ final class WordPressOrgRequestTests: XCTestCase {
let url = try XCTUnwrap(request.asURLRequest().url)

// Then
let expectedURL = "http://test.com/wp-json/test/request"
let expectedURL = "https://test.com/wp-json/test/request"
assertEqual(url.absoluteString, expectedURL)
}

Expand Down
26 changes: 10 additions & 16 deletions WooCommerce/WooCommerceTests/Authentication/AuthenticationManagerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import Yosemite

/// Test cases for `AuthenticationManager`.
final class AuthenticationManagerTests: XCTestCase {

let testSiteURL = "https://test.com"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

DRYed the value while at it.

DRYing literals in unit tests can sometimes backfire because it reduces readability, or rather how expressive the tests are.

In this case, though, the URL is not used in any assertion so there's no loss of clarity.


/// We do not allow automatic WPCOM account sign-up if the user entered an email that is not
/// registered in WordPress.com. This configuration is set up in
/// `WordPressAuthenticatorConfiguration` in `AuthenticationManager.initialize()`.
Expand Down Expand Up @@ -158,15 +161,14 @@ final class AuthenticationManagerTests: XCTestCase {
func test_it_shows_error_upon_login_epilogue_if_the_self_hosted_site_does_not_have_jetpack() {
// Given
let manager = AuthenticationManager()
let testSite = "http://test.com"
let siteInfo = siteInfo(url: testSite,
let siteInfo = siteInfo(url: testSiteURL,
exists: true,
hasWordPress: true,
isWordPressCom: false,
hasJetpack: false,
isJetpackActive: false,
isJetpackConnected: false)
let wpcomCredentials = WordPressComCredentials(authToken: "abc", isJetpackLogin: false, multifactor: false, siteURL: testSite)
let wpcomCredentials = WordPressComCredentials(authToken: "abc", isJetpackLogin: false, multifactor: false, siteURL: testSiteURL)
let credentials = AuthenticatorCredentials(wpcom: wpcomCredentials, wporg: nil)
let navigationController = UINavigationController()

Expand All @@ -185,15 +187,14 @@ final class AuthenticationManagerTests: XCTestCase {
func test_it_shows_account_mismatch_upon_login_epilogue_if_the_site_has_active_jetpack_but_not_connected() {
// Given
let manager = AuthenticationManager()
let testSite = "http://test.com"
let siteInfo = siteInfo(url: testSite,
let siteInfo = siteInfo(url: testSiteURL,
exists: true,
hasWordPress: true,
isWordPressCom: false,
hasJetpack: true,
isJetpackActive: true,
isJetpackConnected: false)
let wpcomCredentials = WordPressComCredentials(authToken: "abc", isJetpackLogin: false, multifactor: false, siteURL: testSite)
let wpcomCredentials = WordPressComCredentials(authToken: "abc", isJetpackLogin: false, multifactor: false, siteURL: testSiteURL)
let credentials = AuthenticatorCredentials(wpcom: wpcomCredentials, wporg: nil)
let navigationController = UINavigationController()

Expand All @@ -209,8 +210,7 @@ final class AuthenticationManagerTests: XCTestCase {
func test_it_can_display_jetpack_error_for_org_site_credentials_sign_in() {
// Given
let manager = AuthenticationManager()
let testSite = "http://test.com"
let siteInfo = WordPressComSiteInfo(remote: ["isWordPress": true, "hasJetpack": false, "urlAfterRedirects": testSite])
let siteInfo = WordPressComSiteInfo(remote: ["isWordPress": true, "hasJetpack": false, "urlAfterRedirects": testSiteURL])
Comment on lines 210 to +213
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test failed 2 out of 2 times in CI 🤔

Maybe there's more work I need to do. Reverting to draft.

let wporgCredentials = WordPressOrgCredentials(username: "cba", password: "password", xmlrpc: "http://test.com/xmlrpc.php", options: [:])
let credentials = AuthenticatorCredentials(wpcom: nil, wporg: wporgCredentials)
let navigationController = UINavigationController()
Expand All @@ -227,15 +227,14 @@ final class AuthenticationManagerTests: XCTestCase {
func test_errorViewController_display_account_mismatch_screen_if_no_site_matches_the_given_self_hosted_site() {
// Given
let manager = AuthenticationManager()
let testSite = "http://test.com"
let navigationController = UINavigationController()
let storage = MockStorageManager()
let matcher = ULAccountMatcher(storageManager: storage)
let wporgCredentials = WordPressOrgCredentials(username: "test", password: "pwd", xmlrpc: "http://test.com/xmlrpc.php", options: [:])
let credentials = AuthenticatorCredentials(wpcom: nil, wporg: wporgCredentials)

// When
let controller = manager.errorViewController(for: testSite, with: matcher, credentials: credentials, navigationController: navigationController) {}
let controller = manager.errorViewController(for: testSiteURL, with: matcher, credentials: credentials, navigationController: navigationController) {}

// Then
XCTAssertNotNil(controller)
Expand All @@ -245,13 +244,12 @@ final class AuthenticationManagerTests: XCTestCase {
func test_errorViewController_returns_account_mismatch_if_no_site_matches_the_given_url() {
// Given
let manager = AuthenticationManager()
let testSite = "http://test.com"
let navigationController = UINavigationController()
let storage = MockStorageManager()
let matcher = ULAccountMatcher(storageManager: storage)

// When
let controller = manager.errorViewController(for: testSite, with: matcher, navigationController: navigationController) {}
let controller = manager.errorViewController(for: testSiteURL, with: matcher, navigationController: navigationController) {}

// Then
XCTAssertNotNil(controller)
Expand All @@ -263,7 +261,6 @@ final class AuthenticationManagerTests: XCTestCase {
let manager = AuthenticationManager()
let navigationController = UINavigationController()

let testSiteURL = "http://test.com"
let testSite = Site.fake().copy(siteID: 1234, name: "Test", url: testSiteURL, isWooCommerceActive: false)

let storage = MockStorageManager()
Expand All @@ -284,7 +281,6 @@ final class AuthenticationManagerTests: XCTestCase {
let manager = AuthenticationManager()
let navigationController = UINavigationController()

let testSiteURL = "http://test.com"
let testSite = Site.fake().copy(siteID: 1234, name: "Test", url: testSiteURL, isWooCommerceActive: true)

let storage = MockStorageManager()
Expand All @@ -303,7 +299,6 @@ final class AuthenticationManagerTests: XCTestCase {
// Given
let navigationController = UINavigationController()

let testSiteURL = "http://test.com"
let testSite = Site.fake().copy(siteID: 1234, name: "Test", url: testSiteURL, isWooCommerceActive: false) // No Woo

let storage = MockStorageManager()
Expand All @@ -326,7 +321,6 @@ final class AuthenticationManagerTests: XCTestCase {
// Given
let navigationController = UINavigationController()

let testSiteURL = "http://test.com"
let testSite = Site.fake().copy(siteID: 1234, name: "Test", url: testSiteURL, isWooCommerceActive: true)

let storage = MockStorageManager()
Expand Down
3 changes: 1 addition & 2 deletions Yosemite/YosemiteTests/Stores/JetpackConnectionStoreTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import XCTest

final class JetpackConnectionStoreTests: XCTestCase {

private let siteURL = "http://test.com"
private let siteURL = "https://test.com"

/// Mock Dispatcher
///
Expand Down Expand Up @@ -229,7 +229,6 @@ final class JetpackConnectionStoreTests: XCTestCase {

func test_fetchJetpackUser_properly_relays_errors() {
// Given
let siteURL = "http://test.com"
let urlSuffix = "/jetpack/v4/connection/data"
let error = NetworkError.unacceptableStatusCode(statusCode: 500)
network.simulateError(requestUrlSuffix: urlSuffix, error: error)
Expand Down