adjust lab stuff

[fadidurah]

No description provided.

[github-actions]

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

[Copilot]

Pull request overview

This PR refactors lab test utilities to use a new static field LabClient.latestLabAccount for storing the most recently fetched/created lab account, replacing the previous pattern of using LabConfig.getCurrentLabConfig().getLabUserPassword(). It also adds support for new user account types (MAM_ON_SPO and CIAM) and includes test coverage for sovereign cloud accounts (US Government and China).

Changes:

• Replaced LabConfig.getCurrentLabConfig() with LabClient.latestLabAccount for accessing test account credentials
• Added MAM_ON_SPO and CIAM to UserType enum and LabConstants
• Added associatedClientId field to LabJsonStringAccountEntry for capturing client association from KeyVault JSON
• Added tests for fetching US Government and China cloud accounts

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
ResourceOwnerPasswordCredentialsTestStrategy.java	Removed LabConfig import and updated getPasswordForUser to use LabClient.latestLabAccount
LabClientTest.java	Added two new test methods canFetchUsGovAccount and canFetchChinaAccount to verify sovereign cloud account retrieval
UserType.java	Added MAM_ON_SPO and CIAM enum values
LabConstants.java	Added corresponding string constants MAM_ON_SPO and CIAM
LabJsonStringAccountEntry.java	Added associatedClientId field with Gson serialization annotation
LabClient.java	Added public static latestLabAccount field and updated three methods to populate it after creating/fetching accounts

[Copilot]

Public static mutable field without synchronization creates race conditions. Multiple threads can invoke getLabAccount, createTempAccountInternal, or getAccountFromLabJsonStringInMobileBuildVault concurrently, leading to non-atomic reads and writes to latestLabAccount. This can result in visibility issues where one thread may not see updates from another thread, or partial state exposure. Consider using AtomicReference<ILabAccount> or synchronization, or better yet, redesign to pass the account through method parameters instead of relying on global state.

Suggested change

	public static ILabAccount latestLabAccount = null;
	public static volatile ILabAccount latestLabAccount = null;

[Copilot]

Potential NullPointerException: direct access to latestLabAccount.getPassword() without null check. If getPasswordForUser is invoked before any lab account has been retrieved or if another thread sets latestLabAccount to null, this will throw a NullPointerException. Add a null check with an appropriate error message or exception type, or ensure that latestLabAccount is guaranteed to be non-null before this method is called.

Suggested change

	String getPasswordForUser(String username) {
	String getPasswordForUser(String username) {
	if (LabClient.latestLabAccount == null) {
	throw new IllegalStateException("No lab account is available to provide a password.");
	}

[Copilot]

Incorrect enum value mapping: MAM_ON_SPO maps to LabConstants.UserType.MAM_CA instead of LabConstants.UserType.MAM_ON_SPO. This creates an ambiguous mapping where both MAM_CA and MAM_ON_SPO enum values resolve to the same string constant "mam_ca". This should map to the newly added LabConstants.UserType.MAM_ON_SPO constant defined at line 41 in LabConstants.java.

Suggested change

	MAM_ON_SPO(LabConstants.UserType.MAM_CA),
	MAM_ON_SPO(LabConstants.UserType.MAM_ON_SPO),

[Copilot]

Using global mutable state instead of parameter passing is an anti-pattern that reduces testability and introduces tight coupling. The latestLabAccount field creates implicit dependencies where calling code must ensure the correct account is set before using methods that depend on it. This makes code harder to reason about, test in isolation, and increases the risk of subtle bugs when multiple tests or operations run concurrently. Consider refactoring to pass the lab account explicitly as a method parameter or through dependency injection.

Suggested change

public static ILabAccount latestLabAccount = null;

[Copilot]

Missing documentation for public static field. This field holds sensitive test account credentials and has significant implications for how the LabClient is used. Add a Javadoc comment explaining: (1) the purpose of this field, (2) when it is updated (after each account fetch/create operation), (3) that it is not thread-safe, (4) that it may be null if no account has been fetched yet, and (5) its intended use case (e.g., for single-threaded test scenarios).

Suggested change

	/**
	* Holds the most recently fetched or created {@link ILabAccount} for this process.
	* <p>
	* This field is updated after each successful account fetch or temporary user
	* creation operation performed by {@link LabClient}. It is intended as a
	* convenience for test code that needs to access the last lab account without
	* explicitly passing it between methods.
	* </p>
	* <p>
	* This field is <strong>not thread-safe</strong>: concurrent reads and writes
	* are not synchronized, and callers must provide their own coordination if it
	* is accessed from multiple threads. The value may be {@code null} if no
	* account has been fetched or created yet, or if the most recent operation
	* failed before updating this field.
	* </p>
	* <p>
	* Intended use is in simple, single-threaded test scenarios only. It should
	* not be relied upon as an authoritative source of account state in
	* multi-threaded or production code.
	* </p>
	*/