test(upload): cover archive validation (#119)

[hardikuppal04]

Summary

Adds hermetic unit coverage for the archive-upload hook's complete pre-upload validation lifecycle: empty selection, maximum-size boundaries, duplicate detection, accepted archive state, rejected-file state, and removal.

This PR does not claim coverage of the backend upload/analysis lifecycle (analyseFile). Issue #119 explicitly scopes the work to validation and state behavior before ingestion.

Linked issue

Closes #119

What changed

Frontend

• Added focused useUpload hook tests.
• Covered empty selections without triggering uploads.
• Covered rejection above the 100 MiB limit and acceptance at exactly MAX_FILE_SIZE.
• Covered the exact user-facing size-limit error.
• Covered case-insensitive duplicate repository-name rejection.
• Covered ZIP and TAR.GZ selected-file state.
• Covered rejectFile and removeFile as separate real UI state transitions.
• Stubbed backendService.uploadRepository and asserted it is never called by validation paths.

Acceptance criteria completed

• Tests cover the maximum-size rejection and exact user-facing error, including acceptance at the exact size boundary.
• Tests cover duplicate repository-name rejection independent of filename casing.
• Tests cover accepted archive selection and the resulting upload state.
• Tests cover rejected-file and remove-file state transitions.
• The tests pass in the repository's standard frontend CI command.

Testing performed

..\..\node_modules\.bin\vitest.cmd run --coverage=false src/features/upload/hooks/useUpload.test.ts
PASS — 1 test file, 8 tests passed.

npm.cmd run lint
PASS — frontend ESLint completed successfully.

npm.cmd run test
PASS — 11 test files, 60 tests passed.

npm.cmd run build
PASS — TypeScript build and Vite production build completed successfully.

Screenshots

Not applicable — this is test-only work with no visible UI change.

Security and data considerations

None. Tests use synthetic File objects, mock the upload backend method, and make no network requests. No credentials, secrets, user data, or generated artifacts are included.

Dependencies and blocked work

None.

Scope changes or remaining work

None. The backend upload/analysis lifecycle is intentionally outside #119's validation-only acceptance criteria and is not claimed as covered by this PR.

Contributor checklist

• This PR targets dev
• I claimed the issue and had it assigned or acknowledged before starting substantial work
• The branch was created from an up-to-date upstream/dev
• The branch is rebased on the latest upstream/dev
• This PR addresses one clearly scoped issue
• Every acceptance criterion I claim as complete is actually complete
• Relevant tests pass
• Documentation is updated for any user-visible change
• No secrets, credentials, local env files, or generated artifacts are included
• No unrelated files were changed
• Closing syntax (Closes) is used only because the issue is fully resolved
• Dependencies and follow-up work are linked

[parthrohit22]

Thanks, @hardikuppal04 — this is clean, well-scoped work. I traced every test against the actual useUpload hook and it all lines up: the size guard and its exact 100 MB string (confirmed via formatFileSize), case-insensitive duplicate-name detection, the accepted-archive uploadFile shape, and the rejectFile / removeFile transitions. I especially like that backendService.uploadRepository is asserted never called on every validation path — that's exactly the right guarantee for pre-ingestion tests. Test hygiene is solid too (vi.hoisted mock, state reset in beforeEach, restoreAllMocks in afterEach, fully hermetic). Branch is properly rebased on dev and CI is green across all jobs.

One small addition before I approve — please fold it into this PR, no separate issue needed:

Add a boundary test at exactly MAX_FILE_SIZE. The tests cover MAX + 1 (rejected) but not the accepted edge. Since the guard is file.size > MAX_FILE_SIZE, a file of exactly 100 MiB should be accepted, and nothing currently pins that. A single case guards against a future > → >= regression that would otherwise pass every existing test, and it completes #119's "maximum-size rejection" criterion. Something like:

it('accepts an archive of exactly the maximum size', () => {
  const uploadRepository = mockUpload();
  const file = archiveFile('at-limit.zip');
  Object.defineProperty(file, 'size', { value: MAX_FILE_SIZE });
  const hook = renderHook(() => useUpload());

  act(() => {
    hook.result.current.selectFile([file]);
  });

  expect(hook.result.current.uploadFile?.size).toBe(MAX_FILE_SIZE);
  expect(hook.result.current.error).toBeNull();
  expect(hook.result.current.success).toBe(true);
  expect(uploadRepository).not.toHaveBeenCalled();
});

Once that's in and CI is green, this is good to merge. Nice work. 🚀

[parthrohit22]

Approving — verified, not just read.

• Every assertion checks out against the hook source: the size-limit message matches formatFileSize(MAX_FILE_SIZE) output exactly ("100 MB"), the boundary pair is correct (MAX_FILE_SIZE accepted, +1 rejected, matching the strict > check), and the duplicate-name test genuinely exercises the case-insensitive .TAR.GZ extension-strip and lowercased name-set lookup rather than echoing a mock.
• Nice touch: the empty-selection test calls rejectFile() first, proving selectFile clears a prior error before its early return.
• Hermeticity is asserted, not assumed — uploadRepository is spied and verified uncalled on every validation path.
• Conventions match the merged useGitHubImport.test.ts pattern exactly (hoisted mock state, repository factory, store reset per test).
• Independently reproduced: applied this file against dev sources locally — 1 file, 8/8 passed.

Scope honesty is appreciated: analyseFile is correctly left out per #119.

One outcome worth noting: reading the hook alongside these tests surfaced a few pre-existing useUpload state quirks (error paths keeping a stale accepted selection, .tgz inconsistency). Those are hook defects, not test gaps, and are now tracked separately in #134.

Two optional nits, non-blocking: a size parameter on archiveFile() would fold in the Object.defineProperty duplication, and the repository() factory is now duplicated across two test files — worth a shared test util the third time it appears.