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

Integrated code lifecycle: Add duration in build queue view #8608

Merged
merged 12 commits into from
May 18, 2024
Merged

Integrated code lifecycle: Add duration in build queue view #8608

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f66a6c0
- Add buildjob duration to queue view
May 16, 2024
a475cac
force duration UI update
BBesrour May 16, 2024
a7f075f
add highlight when duration longer than 240
BBesrour May 16, 2024
112b831
reduce interval to 1s
BBesrour May 16, 2024
ee415d5
add tests
BBesrour May 16, 2024
0e9aae7
reduce width
BBesrour May 16, 2024
633d776
fix name and reduce build duration width
BBesrour May 16, 2024
16e186e
improve readability
BBesrour May 16, 2024
515c87e
change requests
BBesrour May 16, 2024
5a161cb
Merge branch 'develop' into feature/integrated-code-lifecycle/add-bui…
BBesrour May 16, 2024
9ff36b4
Merge branch 'develop' into feature/integrated-code-lifecycle/add-bui…
BBesrour May 16, 2024
ec2ae86
Merge branch 'develop' into feature/integrated-code-lifecycle/add-bui…
BBesrour May 17, 2024
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
24 changes: 22 additions & 2 deletions src/main/webapp/app/localci/build-queue/build-queue.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,17 @@ <h3 id="build-queue-running-heading" jhiTranslate="artemisApp.buildAgents.runnin
<fa-icon [icon]="controls.iconForSortPropField('id')" />
</span>
</ng-template>
<ng-template ngx-datatable-cell-template let-value="value">
<span>{{ value }}</span>
<ng-template ngx-datatable-cell-template let-value="value" let-row="row">
<span
[ngClass]="{
'text-danger': row.jobTimingInfo.buildDuration > 240
}"
>
@if (row.jobTimingInfo.buildDuration > 240) {
<fa-icon [icon]="faExclamationCircle" />
}
{{ value }}
</span>
</ng-template>
</ngx-datatable-column>
<ngx-datatable-column prop="buildAgentAddress" [minWidth]="150" [width]="200">
Expand Down Expand Up @@ -146,6 +155,17 @@ <h3 id="build-queue-running-heading" jhiTranslate="artemisApp.buildAgents.runnin
}
</ng-template>
</ngx-datatable-column>
<ngx-datatable-column prop="jobTimingInfo.buildDuration" [minWidth]="150" [width]="150">
BBesrour marked this conversation as resolved.
Show resolved Hide resolved
<ng-template ngx-datatable-header-template>
<span class="datatable-header-cell-wrapper" (click)="controls.onSort('jobTimingInfo.buildDuration')">
<span class="datatable-header-cell-label bold sortable" jhiTranslate="artemisApp.buildQueue.buildJob.buildDuration"></span>
<fa-icon [icon]="controls.iconForSortPropField('jobTimingInfo.buildDuration')" />
</span>
</ng-template>
<ng-template ngx-datatable-cell-template let-value="value">
<span>{{ value | artemisDurationFromSeconds }}</span>
</ng-template>
</ngx-datatable-column>
<ngx-datatable-column prop="jobTimingInfo.submissionDate" [minWidth]="150" [width]="200">
<ng-template ngx-datatable-header-template>
<span class="datatable-header-cell-wrapper" (click)="controls.onSort('jobTimingInfo.submissionDate')">
Expand Down
24 changes: 24 additions & 0 deletions src/main/webapp/app/localci/build-queue/build-queue.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export class BuildQueueComponent implements OnInit, OnDestroy {
page = 1;
predicate = 'build_completion_date';
ascending = false;
interval: any;
BBesrour marked this conversation as resolved.
Show resolved Hide resolved

constructor(
private route: ActivatedRoute,
Expand All @@ -49,6 +50,9 @@ export class BuildQueueComponent implements OnInit, OnDestroy {

ngOnInit() {
this.loadQueue();
this.interval = setInterval(() => {
this.updateBuildJobDuration();
}, 1000);
this.loadFinishedBuildJobs();
this.initWebsocketSubscription();
}
Expand All @@ -62,6 +66,7 @@ export class BuildQueueComponent implements OnInit, OnDestroy {
this.courseChannels.forEach((channel) => {
this.websocketService.unsubscribe(channel);
});
clearInterval(this.interval);
}

/**
Expand Down Expand Up @@ -259,4 +264,23 @@ export class BuildQueueComponent implements OnInit, OnDestroy {
refresh() {
this.loadFinishedBuildJobs();
}

/**
* Update the build jobs duration
*/
updateBuildJobDuration() {
if (!this.runningBuildJobs) {
return;
}

for (const buildJob of this.runningBuildJobs) {
if (buildJob.jobTimingInfo && buildJob.jobTimingInfo?.buildStartDate) {
const start = dayjs(buildJob.jobTimingInfo?.buildStartDate);
const now = dayjs();
buildJob.jobTimingInfo.buildDuration = now.diff(start, 'seconds');
}
}
// This is necessary to update the view when the build job duration is updated
this.runningBuildJobs = JSON.parse(JSON.stringify(this.runningBuildJobs));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the proper way would be to trigger the change detection.
Or at least only create a new array instead of parsing it: this.runningBuildJobs = [...this.runningBuildJobs];

Copy link
Member Author

@BBesrour BBesrour May 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

These unfortunately didn't work when I tried them

  • ref.detectChanges() doesn't work due to how ngx-datatable is implemented
  • this.runningBuildJobs = [...this.runningBuildJobs]; didn't work too (although they say that it should work in their docs)

Copy link
Contributor

Choose a reason for hiding this comment

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

The spread operator ('...') is a shallow copy method, which means it copies the elements of the array but not the nested objects themselves, they remain referenced to the original objects.
The JSON.parse(...) is a deep copying method and creates entirely new and independent objects.
I did not go into detail on BuildJobs -Class, but this could explain why only the parsing works correctly for your runningBuildJobs array

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we already use eslint could we not just use the deep copy method of that library?
Also I'm not completely sure but could setting it to an empty array first and then re-setting it work?

}
}
18 changes: 18 additions & 0 deletions src/test/javascript/spec/component/localci/build-queue/build-queue.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,24 @@ describe('BuildQueueComponent', () => {
expect(spy).toHaveBeenCalled();
});

it('should update build job duration in running build jobs', () => {
// Mock ActivatedRoute to return no course ID
mockActivatedRoute.paramMap = of(new Map([]));

// Mock BuildQueueService to return mock data
mockBuildQueueService.getRunningBuildJobs.mockReturnValue(of(mockRunningJobs));

// Initialize the component
component.ngOnInit();
// Expectations: The build job duration is calculated and set for each running build job
for (const runningBuildJob of component.runningBuildJobs) {
const { buildDuration, buildCompletionDate, buildStartDate } = runningBuildJob.jobTimingInfo!;
if (buildDuration && buildCompletionDate && buildStartDate) {
expect(buildDuration).toBeLessThanOrEqual(buildCompletionDate.diff(buildStartDate, 'seconds'));
}
}
});

it('should cancel a build job in a course', () => {
const buildJobId = '1';

Expand Down
Loading