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.

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

Jump to bottom

User/ujsrivastava/block unblock device test spec #672

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

User/ujsrivastava/block unblock device test spec #672

wants to merge 8 commits into from
+258 −5

Conversation

ASPS002
Copy link
Collaborator

@ASPS002 ASPS002 commented Feb 19, 2025 •
edited
Loading

Description

This Pull Request adds two properties to the requestHydraLabTest test configuation for blocking and unblocking a device on which the test runs.

Blocking a device by a task
Input:
A boolean property blockDevice needs to be provided in the requestHydraLabTest Gradle task input. When it is passed as true, the deviceIdentifier passed in the task is blocked as follows:

  • If the deviceeIdentifier corresponds to a device group, then it blocks any one of the available online unblocked devices from the device group and the test runs on that device.
  • If the deviceIdentifier corresponds to a specific device, then it blocks that specific device and the test runs on that specific device.

Output:

  • Outputs BlockedDeviceSerialNumber that can be used in the next task and used for further task runs on the same device or any further actions.
  • Outputs UnblockDeviceSecretKey that can be used in the next task in which you want to unblock the device usinig this secret key.

Unblocking a device by a task
Input:
A boolean property unblockDevice and unblockDeviceSecretKey needs to be provided in the requestHydraLabTest Gradle task input. When the unblockDevice is passed as true and unblockDeviceSecretKey is not empty , then the deviceIdentifier passed in the task is unblocked as follows:

  • The deviceIdentifier should correspond to a specific device and not a group, if so it then runs task on that device and unblocks that specific device , so that it can be used again by other tasks.

Unblocking frozen blocked devices
Automatically unblocking of the devices which have been blocked for more than 4 hours. The default time for unblocking a device is taken as 4 hours for now.

Usecase
This is useful in cases when you want to run multiple tasks on the same device one after the other.

typeSpecificParamCheck
Added the following type specific param check for runningType INSTRUMENTATION:

  • Added check that a task should not set both unblockDevice and blockDevice as true in the same task as this is always true by default and redundant.
  • Added check that when unblockDevice is set to true the value of unblockDeviceSecretKey should not be empty as it is required to unblock a specific device blocked by a specific task.
  • Added check that when unblockDevice is set to true the value of deviceIdentifier passed for blocking should not be a group and instead it should be a specific device serial number.

Linked GitHub issue ID: #673

Pull Request Checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Code compiles correctly with all tests are passed.
  • I've read the contributing guide and followed the recommended practices.
  • Wikis or README have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

If this introduces a breaking change for Hydra Lab users, please describe the impact and migration path.

  • Yes
  • No

How you tested it

Tested it on the center side by sending a POST api api/test/task/run with required parameters. It successfully blocks and unblocks the device successfully. This can be validated from the test task id as well. Here are the screenshots of the same.

Blocking:
Screenshot 2025-02-27 at 11 49 46

Unblocking:
Screenshot 2025-02-27 at 11 52 38

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Technical design
  • Build related changes
  • Refactoring (no functional changes, no api changes)
  • Code style update (formatting, renaming) or Documentation content changes
  • Other (please describe):

Feature UI screenshots or Technical design diagrams

If this is a relatively large or complex change, kick it off by drawing the tech design with PlantUML and explaining why you chose the solution you did and what alternatives you considered, etc...

Ujjwal Srivastava added 3 commits February 18, 2025 16:58
Added blockDevice and unblockDevice in the TestConfig property. Added…
1b09602 
… methods in DeviceAgentManagementService to handle blocking and unblocking a device and releasing any frozen blocked devices
Added check that block and unblock device should not be true in the s…
61dab02 
…ame test task. Added block device by specific device Identifier
added type specific param check when unblock device is true and devic…
0f723cc 
…eIdentifier is a group. Also changed the name of output variable unblockDeviceSecretKey to UnblockDeviceSecretKey
@ASPS002
Copy link
Collaborator Author

ASPS002 commented Feb 19, 2025

@microsoft-github-policy-service agree company="Microsoft"

@ASPS002 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"

@ASPS002 ASPS002 self-assigned this Feb 19, 2025
@ASPS002 ASPS002 marked this pull request as ready for review February 21, 2025 05:06
zhou9584
zhou9584 reviewed Feb 24, 2025
View reviewed changes
center/src/main/java/com/microsoft/hydralab/center/service/DeviceAgentManagementService.java Outdated
@@ -957,23 +978,34 @@ private JSONObject runTestTaskByGroup(TestTaskSpec testTaskSpec) {
}

private JSONObject runTestTaskByDevice(TestTaskSpec testTaskSpec) {
if (testTaskSpec.unblockDevice) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If the user wants to block the device and then run ​​3 tasks, an error will occur. Secret detection and unlocking the device should be separated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a check if (device.isTesting() || (!isRunOnBlockedDevice(testTaskSpec) && isDeviceBlocked(testTaskSpec.deviceIdentifier))), which checks if a task wants to run a test on a blocked device then it should be able to run the test on that blocked device if the secret key matches using the method isRunOnBlockedDevice(TestTaskSpec testTaskSpec) .

Later on we add a check if the task wants to unblock the device. If so it unblocks the device as well. Thus now even if the user wants to run multiple tests on a particular device without unblocking it, the user can do the same. Request to review the logic.

@ASPS002 ASPS002 removed their assignment Feb 24, 2025
@rakirs2
Copy link

rakirs2 commented Feb 24, 2025 via email

@girmaw
Copy link

girmaw commented Feb 24, 2025 via email

@shaleen
Copy link

shaleen commented Feb 24, 2025

Repo owners: Please figure out how to set up notifications properly and stop sending notifications.

@losamig
Copy link

losamig commented Feb 24, 2025 via email

@bpkroth
Copy link

bpkroth commented Feb 24, 2025

image Somehow all/many MS employees were subscribed to "All Activity" for this repo at some point. You can unselect for yourself in the image above, but yes, it'd be nice if folks didn't do this in the first place 🙏 😄

Ujjwal Srivastava and others added 5 commits February 25, 2025 12:21
added support to run test on blocked devices. The task needs to provi…
90192d4 
…de the unblockDeviceSecretKey along with the device serial number to run a task on the blocked device with the specific device serial number.
unblock a device after its status has been updated to testing by the …
e949b99 
…task.
fixed failures in task :center:checkstyleMain
82dd16a
fixed failing tests runTestOnDeviceWithApp
08174fa
Added data members for Task class and added logic to retry task on th…
0e35af8 
…e blocked device itself if that task blocks that device earlier.
zhou9584
zhou9584 reviewed Feb 27, 2025
View reviewed changes
center/src/main/java/com/microsoft/hydralab/center/service/DeviceAgentManagementService.java
if (isAgentUpdating(agentSessionInfoByAgentId.agentUser.getId())) {
return result;
}
updateDeviceStatus(device.getSerialNum(), DeviceInfo.TESTING, testTaskSpec.testTaskId);

if (testTaskSpec.unblockDevice) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you need to add a validation before: unblockDevice and blockDevice could not be true value at the same time

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also had similar point of view. And that is why already added that check in the groovy/com/microsoft/hydralab/ClientUtilsPlugin.groovy file. I think these would be sufficient. Let me know your thoughts on it.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

The gradle plugin is not the only way to access hydra lab, so we need to verify it again on the Center side.

Copy link
Contributor

Choose a reason for hiding this comment

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

Call center is also possible via Azure Extension or REST API driectly

olivershen-wow
olivershen-wow approved these changes Feb 27, 2025
View reviewed changes
Copy link
Contributor

@olivershen-wow olivershen-wow left a comment

Choose a reason for hiding this comment

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

LGTM for the plugin part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhou9584 zhou9584 zhou9584 left review comments

@olivershen-wow olivershen-wow olivershen-wow approved these changes

@taoran6 taoran6 Awaiting requested review from taoran6 taoran6 is a code owner

@DexterDreeeam DexterDreeeam Awaiting requested review from DexterDreeeam DexterDreeeam is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ASPS002 @rakirs2 @girmaw @shaleen @losamig @bpkroth @zhou9584 @olivershen-wow