Run only one measurement at a time 2.0.5

src/app/services/measurement-client.service.spec.ts

@@ -178,4 +178,87 @@ describe('MeasurementClientService ndt7 integration', () => {
 
     expect(ndt7TestSpy).toHaveBeenCalledTimes(1);
   });
+
+  describe('single-flight lock', () => {
+    /** Holds the ndt7 run open so a second runTest() overlaps it. */
+    const blockNdt7 = () => {
+      let release: () => void;
+      const started = new Promise<void>((resolve) => {
+        ndt7TestSpy.and.callFake(
+          () => new Promise<number>((done) => {
+            resolve();
+            release = () => done(0);
+          })
+        );
+      });
+      return { started, release: () => release() };
+    };
+
+    it('drops a second run started while one is in flight', async () => {
+      const gate = blockNdt7();
+      const first = service.runTest('manual');
+      await gate.started;
+
+      // Second tap on the test button, first run still going.
+      await service.runTest('manual');
+      expect(ndt7TestSpy).toHaveBeenCalledTimes(1);
+
+      gate.release();
+      await first;
+      expect(ndt7TestSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('blocks the manual button path while the first test runs', async () => {
+      const gate = blockNdt7();
+      const first = service.runTest('first');
+      await gate.started;
+
+      // What the user hit: the post-registration test was still going.
+      await service.runTest('manual');
+      expect(ndt7TestSpy).toHaveBeenCalledTimes(1);
+
+      gate.release();
+      await first;
+    });
+
+    it('holds off a scheduled run while a manual one is going', async () => {
+      const gate = blockNdt7();
+      const manual = service.runTest('manual');
+      await gate.started;
+
+      await service.runTest('daily', { slot: 'A', scheduledAt: 1 });
+      expect(ndt7TestSpy).toHaveBeenCalledTimes(1);
+
+      gate.release();
+      await manual;
+      // Link free again: the scheduler's next tick gets to run.
+      expect(service.isRunning).toBeFalse();
+    });
+
+    it('releases the lock once the run finishes', async () => {
+      await service.runTest('manual');
+      await service.runTest('manual');
+
+      expect(ndt7TestSpy).toHaveBeenCalledTimes(2);
+    });
+
+    it('releases the lock when the run fails', async () => {
+      ndt7TestSpy.and.rejectWith(new Error('websocket closed unexpectedly'));
+
+      await service.runTest('manual');
+      expect(service.isRunning).toBeFalse();
+
+      await service.runTest('manual');
+      expect(ndt7TestSpy).toHaveBeenCalledTimes(2);
+    });
+
+    it('reports the running state through testRunning$', async () => {
+      const seen: boolean[] = [];
+      service.testRunning$.subscribe((v) => seen.push(v));
+
+      await service.runTest('manual');
+
+      expect(seen).toEqual([false, true, false]);
+    });
+  });
 });

src/app/services/measurement-client.service.ts

@@ -17,10 +17,18 @@ export class MeasurementClientService {
   public uploadComplete$ = new Subject<any>();
   public downloadStarted$ = new Subject<any>();
   public uploadStarted$ = new Subject<any>();
+  /** true while a measurement is in flight; drives the disabled state of the test button. */
+  public testRunning$ = new BehaviorSubject<boolean>(false);
 
   private TIME_EXPECTED = 10;
   private retryAttempts = 0;
   private maxRetries = 3;
+  /**
+   * The run currently in flight, or null. Two ndt7 runs over the same link steal
+   * bandwidth from each other and both report low, so only one is allowed at a
+   * time — see runTest().
+   */
+  private activeRun: Promise<void> | null = null;
   private readonly measurementNotificationActivity = new BehaviorSubject<any>(
     {}
   ).asObservable();
@@ -80,13 +88,43 @@ export class MeasurementClientService {
     private deviceContext: DeviceContextService
   ) {}
 
+  /** Whether a measurement is running right now. */
+  get isRunning(): boolean {
+    return this.activeRun !== null;
+  }
+
+  /**
+   * Runs one ndt7 measurement, single-flight.
+   *
+   * Every trigger funnels through here — the test button, the post-registration
+   * 'first' test, the scheduled slots and the startup test — and any of them can
+   * fire while another is still running. Two runs sharing the link measure each
+   * other's traffic as congestion, so both upload speeds that are too low.
+   *
+   * A request that arrives while a run is in flight is dropped, not queued: a
+   * measurement that starts late is worth less than the one already running, and
+   * queueing would just move the overlap later.
+   */
   async runTest(
     notes = 'manual',
     scheduleContext: { slot: string | null; scheduledAt: number | null } = null
   ): Promise<void> {
+    if (this.activeRun) {
+      console.warn(
+        `Measurement already in progress; dropping the "${notes}" request.`
+      );
+      return;
+    }
     console.log('Starting ndt7 test', ndt7);
     this.retryAttempts = 0;
-    await this.runTestWithRetry(notes, scheduleContext);
+    this.testRunning$.next(true);
+    this.activeRun = this.runTestWithRetry(notes, scheduleContext);
+    try {
+      await this.activeRun;
+    } finally {
+      this.activeRun = null;
+      this.testRunning$.next(false);
+    }
   }
 
   private async runTestWithRetry(

src/app/services/schedule.service.spec.ts

@@ -45,6 +45,7 @@ describe('ScheduleService', () => {
     };
     measurementClientService = {
       runTest: jasmine.createSpy('runTest').and.resolveTo(undefined),
+      isRunning: false,
     };
     settingsService = {
       get: jasmine.createSpy('get').and.resolveTo(true),
@@ -239,4 +240,29 @@ describe('ScheduleService', () => {
       expect(sem.choice).toBeLessThanOrEqual(sem.end);
     });
   });
+
+  describe('waiting for a running measurement', () => {
+    it('leaves the semaphore alone so the next tick retries', async () => {
+      measurementClientService.isRunning = true;
+      const sem = slotASemaphore({ choice: NOW.getTime() - MINUTE });
+      await service.setSemaphore(sem);
+
+      await service.decide(savedSemaphore());
+
+      expect(measurementClientService.runTest).not.toHaveBeenCalled();
+      // Untouched: same choice, no retry counters moved.
+      expect(savedSemaphore().choice).toBe(sem.choice);
+      expect(savedSemaphore().retryAttempts).toBe(0);
+    });
+
+    it('runs once the other measurement has finished', async () => {
+      measurementClientService.isRunning = false;
+      const sem = slotASemaphore({ choice: NOW.getTime() - MINUTE });
+      await service.setSemaphore(sem);
+
+      await service.decide(savedSemaphore());
+
+      expect(measurementClientService.runTest).toHaveBeenCalledTimes(1);
+    });
+  });
 });

src/app/services/schedule.service.ts

@@ -170,6 +170,13 @@ export class ScheduleService {
         await this.setSemaphore({});
         return;
       }
+      if (this.measurementClientService.isRunning) {
+        // A manual or first-run test is using the link. Leave the semaphore
+        // untouched and let the next tick try again, so the scheduled test
+        // waits for the running one instead of being lost.
+        console.log('Another measurement is running, waiting for it to finish.');
+        return;
+      }
       if (scheduleSemaphore.lastFailReason === 'no-network') {
         // Network is back: the failed-test backoff starts over
         scheduleSemaphore.backoffLevel = 0;

src/app/shared/circular-progress-bar/circular-progress-bar.component.ts

@@ -15,11 +15,19 @@ export class CircularProgressBarComponent {
   @Input() currentRateDownload!: number;
   @Input() error: boolean = false;
   @Input() completed: boolean = false;
+  /** Swallows clicks while a measurement is running, so a second tap cannot start one. */
+  @Input() disabled: boolean = false;
 
   @Output() startTest = new EventEmitter<void>();
   @Output() showError = new EventEmitter<boolean>();
 
   handleClick() {
+    // A run in progress passes the checks below at its start and at its end
+    // (progressValue 0 and 100), so the disabled flag is what actually stops a
+    // second test from being launched on top of the first.
+    if (this.disabled) {
+      return;
+    }
     if (
       this.progressValue === 0 ||
       this.progressValue === 100 ||

src/app/starttest/starttest.page.html

@@ -89,6 +89,7 @@ <h2 class="title" title="{{school?.name}}">{{school?.name}}</h2>
         [statusMessage]="getStatusMessage()"
         [icon]="''"
         [error]="(connectionStatus=='error' && currentState == undefined) || (!onlineStatus)"
+        [disabled]="testInProgress"
         (startTest)="startNDT()"
         [firstLabel]="(currentState == undefined && currentRate != 'error' && onlineStatus) ? ('startTest.startTest' | translate) : ''"
         [currentRateDownload]="currentRateDownload"

src/app/starttest/starttest.page.ts

@@ -104,6 +104,9 @@ export class StarttestPage implements OnInit, OnDestroy {
   private uploadSub!: Subscription;
   private downloadStartedSub!: Subscription;
   private uploadStartedSub!: Subscription;
+  private testRunningSub!: Subscription;
+  /** Mirrors MeasurementClientService.testRunning$; disables the test button while a run is live. */
+  testInProgress = false;
 
   downloadTimer: any;
   uploadTimer: any;
@@ -286,6 +289,13 @@ export class StarttestPage implements OnInit, OnDestroy {
   private setupServiceSubscriptions() {
     console.log('Setting up service subscriptions');
 
+    this.testRunningSub = this.measurementClientService.testRunning$.subscribe(
+      (running) => {
+        this.testInProgress = running;
+        this.ref.markForCheck();
+      }
+    );
+
     this.downloadSub =
       this.measurementClientService.downloadComplete$.subscribe((data) => {
         this.downloadStarted = false;
@@ -565,6 +575,13 @@ export class StarttestPage implements OnInit, OnDestroy {
   }
 
   startNDT(notes: string = 'manual') {
+    // Bail out before touching the UI state. The service drops the duplicate run
+    // anyway, but everything below resets the progress of the test that is still
+    // running, which makes the app look stuck and invites another tap.
+    if (this.testInProgress) {
+      console.warn(`Test already running; ignoring the "${notes}" request.`);
+      return;
+    }
     try {
       this.uploadProgressStarted = false;
       this.downloadStarted = false;
@@ -1015,5 +1032,6 @@ export class StarttestPage implements OnInit, OnDestroy {
     this.uploadSub.unsubscribe();
     this.downloadStartedSub.unsubscribe();
     this.uploadStartedSub.unsubscribe();
+    this.testRunningSub?.unsubscribe();
   }
 }