Adding parent-child job support to the pipeline job. by sanjay000001 · Pull Request #92 · jenkinsci/browserstack-integration-plugin

sanjay000001 · 2025-06-04T07:25:27Z

Testing done

Submitter checklist

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests that demonstrate the feature works or the issue is fixed

agrawalsaurabhs · 2025-06-18T05:03:23Z

...a/com/browserstack/automate/ci/jenkins/qualityDashboard/QualityDashboardPipelineTracker.java

-                            apiUtil.logToQD(browserStackCredentials, "Finally no artifacts found for jobName: " + jobName + " and buildNumber: " + buildNumber);
-                        }
-                        sendBuildDataToQD(run, overallResult, qdS3Url, browserStackCredentials);
+                        processArtifactsAndSendData(run, overallResult, browserStackCredentials, jobName, buildNumber);


Just for clean code.
String jobName = getJobNameFromRun(run, browserStackCredentials);
int buildNumber = run.getNumber();

These 2 lines can be moved inside processArtifactsAndSendData since we are already passing run object to the method.

Actually, the build name is needed to check whether the pipeline is enabled. Based on that, we call the processArtifactsAndSendData function. The build number is used to improve logging and help uniquely identify the run in the try-catch block.

agrawalsaurabhs · 2025-06-18T05:23:00Z

...ain/java/com/browserstack/automate/ci/jenkins/qualityDashboard/UpstreamPipelineResolver.java

+     * @param browserStackCredentials The credentials required for logging
+     * @param message The error message to log
+     */
+    private static void logError(BrowserStackCredentials browserStackCredentials, String message) {


looks like wrapper over a wrapper. any specific reason of creating this?

We handle error logging this way to avoid declaring throws JsonProcessingException in every function. Otherwise, we would also need to add extra try-catch blocks at the top level just to log the error when calling the Upstream resolver methods(as seen in QualityDashboardPipelineTracker.java, lines 53–57). This approach reduces repetition and keeps error logging centralized in UpStreamResolver.

agrawalsaurabhs · 2025-06-18T05:28:42Z

...ain/java/com/browserstack/automate/ci/jenkins/qualityDashboard/UpstreamPipelineResolver.java

+            if (cause instanceof Cause.UpstreamCause) {
+                return handleUpstreamCause((Cause.UpstreamCause) cause, browserStackCredentials, visitedProjects);
+            } else if (cause instanceof hudson.triggers.TimerTrigger.TimerTriggerCause) {
+                apiUtil.logToQD(browserStackCredentials, "Build triggered by timer/schedule");


should we just log it on Jenkins side? is this log useful for debugging on server side?

agrawalsaurabhs · 2025-06-18T05:30:59Z

src/main/java/com/browserstack/automate/ci/common/constants/Constants.java

        public static final String QEI_DEFAULT_URL = "https://quality-engineering-insights.browserstack.com";
        public static String host = QEI_DEFAULT_URL;
+        // Cache configuration
+        public static final long CACHE_DURATION_MS = 60 * 60 * 1000L; // 1 hour in milliseconds


Ok for now but ideally it should be configurable either on client side or server side.

Noted. As discussed, we will handle this in the future.

agrawalsaurabhs

LGTM! clean code, nicely done.
minor comments added.

karanshah-browserstack

All file changes, except those in common, fall under QD’s ownership, an internal review within QD should be sufficient. Also, can we update the CODEOWNERS file to reflect QD as the owner for these files? I won’t be able to review them as I don’t have enough context.

karanshah-browserstack · 2025-06-19T08:12:38Z

src/main/java/com/browserstack/automate/ci/jenkins/qualityDashboard/QualityDashboardInit.java

                } catch (JsonProcessingException e) {
                    // Handling the exception and logging an error
-                    System.err.println("Error processing JSON for job: " + job.getName());
+                    LOGGER.info("Error processing JSON for job: " + job.getName() + " - " + e.getMessage());


These should be logger error right? And till will also be displayed to user correct?

@karanshah-browserstack , yes, the user will be able to see it in the Jenkins System Log. I've changed it to a warning in all places.

karanshah-browserstack · 2025-06-19T08:12:59Z

src/main/java/com/browserstack/automate/ci/jenkins/qualityDashboard/QualityDashboardInit.java

                apiUtil.logToQD(browserStackCredentials, "Issue getting Jenkins Instance");
            } catch (JsonProcessingException e) {
-                System.err.println("Error logging issue with Jenkins instance.");
+                LOGGER.info("Error logging issue with Jenkins instance.");


same error Logger comment as above for all logger related changes

…supporting job and folder item type handling on rename and deletion.

sanjay000001 · 2025-06-19T11:58:01Z

@karanshah-browserstack , we’ll review and update the CODEOWNERS for QD related files.

karanshah-browserstack · 2025-06-26T06:56:13Z

src/main/java/com/browserstack/automate/ci/jenkins/qualityDashboard/QualityDashboardInit.java

                apiUtil.logToQD(browserStackCredentials, "Issue getting Jenkins Instance");
            } catch (JsonProcessingException e) {
-                LOGGER.info("Error logging issue with Jenkins instance.");
+                LOGGER.warning("Error logging issue with Jenkins instance.");


Shouldn't this be error?

@karanshah-browserstack, We are logging this as a warning instead of an error because the logs appear on the user's Jenkins system. We want to avoid showing error logs to the user.

sanjay000001 added 8 commits January 13, 2025 17:26

[maven-release-plugin] prepare release bstack

41efd1f

[maven-release-plugin] prepare for next development iteration

9df77f8

Merge branch 'master' of https://github.com/jenkinsci/browserstack-in…

6abe177

…tegration-plugin

[maven-release-plugin] prepare release browserstack-integration-1.2.19

cc13d94

[maven-release-plugin] prepare for next development iteration

691cdaa

Parent child relationship

cd1fb7c

Reverting the pom.xml file changes

c1f4865

Adding a try-catch while resolving parent-child relationship.

a8fcb9f

akshit9738 approved these changes Jun 4, 2025

View reviewed changes

sanjay000001 added 3 commits June 7, 2025 00:57

Added Support for Multi-Configuration/Matrix Job and artifacts handling

b1d7a80

Adding Caching for QD Enabled and check for Creds

5056c1a

Using Instant time module for cache expiry.

bddc072

akshit9738 approved these changes Jun 12, 2025

View reviewed changes

Replacing System.print to Logger.info

5df30d4

agrawalsaurabhs reviewed Jun 18, 2025

View reviewed changes

sanjay000001 added 2 commits June 19, 2025 01:36

Sending the job type to QEI.

25cc14b

Adding try-catch around the getItemType

bea898d

karanshah-browserstack suggested changes Jun 19, 2025

View reviewed changes

Sending job type at the initials set-up and item creation steps, and …

e2cad42

…supporting job and folder item type handling on rename and deletion.

sanjay000001 requested a review from karanshah-browserstack June 19, 2025 08:35

changing logger method from info to warning.

631cf69

karanshah-browserstack suggested changes Jun 26, 2025

View reviewed changes

karanshah-browserstack approved these changes Jun 26, 2025

View reviewed changes

shandli123 approved these changes Jul 9, 2025

View reviewed changes

francisf approved these changes Jul 9, 2025

View reviewed changes

francisf merged commit 1be5219 into jenkinsci:master Jul 9, 2025
1 check passed

Uh oh!

Conversation

sanjay000001 commented Jun 4, 2025

Testing done

Submitter checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agrawalsaurabhs left a comment

Choose a reason for hiding this comment

Uh oh!

karanshah-browserstack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanjay000001 commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants