ocom-verify package changes#242
Open
jasonmorais wants to merge 41 commits into
Open
Conversation
…ome, and using the dev call of the actual servers, like in the case of auth, to further decrease decoupling
Contributor
Reviewer's GuideRefactors the verification test infrastructure to use centralized timeout settings and new test server abstractions, tightens the ocom-verification coverage and e2e pipelines, and adds richer community-create UI/API/e2e tests with more robust GraphQL and UI handling while scoping coverage merging to the verification package. Sequence diagram for community create acceptance UI flowsequenceDiagram
actor Tester
participant CucumberRunner
participant StepDefinitions
participant SerenityActor
participant ReactRender
participant CommunityCreateComponent
participant CreateCommunityTask
participant JsdomPageAdapter
participant CommunityPage
participant CommunityUiNotes
Tester->>CucumberRunner: run_scenario
CucumberRunner->>StepDefinitions: execute_Given_is_an_authenticated_community_owner
StepDefinitions->>SerenityActor: actorCalled(actorName)
StepDefinitions->>ReactRender: mountComponent(CommunityCreate)
ReactRender-->>StepDefinitions: rendered_container
StepDefinitions->>CreateCommunityTask: setContainer(container)
StepDefinitions->>SerenityActor: set_initial_CommunityUiNotes
SerenityActor-->>StepDefinitions: notes_initialized
CucumberRunner->>StepDefinitions: execute_When_creates_a_community_with
StepDefinitions->>SerenityActor: attemptsTo(CreateCommunityTask)
SerenityActor->>CreateCommunityTask: run(name)
CreateCommunityTask->>CreateCommunityTask: getContainer()
CreateCommunityTask->>JsdomPageAdapter: new(container)
CreateCommunityTask->>CommunityPage: new(adapter)
CreateCommunityTask->>CommunityPage: fillName(name)
CommunityPage->>JsdomPageAdapter: fill(selector,name)
CreateCommunityTask->>CommunityPage: clickCreate()
CommunityPage->>JsdomPageAdapter: click(selector)
CreateCommunityTask->>CreateCommunityTask: flushAsync()
par React_onSubmit_chain
CommunityCreateComponent-->>StepDefinitions: onSave(values)
StepDefinitions->>SerenityActor: attemptsTo(update_CommunityUiNotes)
SerenityActor-->>StepDefinitions: notes_updated
end
CucumberRunner->>StepDefinitions: execute_Then_community_should_be_created_successfully
StepDefinitions->>SerenityActor: answer(CommunityCreatedFlag)
SerenityActor->>CommunityUiNotes: read(formSubmitted)
CommunityUiNotes-->>SerenityActor: true_or_false
SerenityActor-->>StepDefinitions: submitted_flag
StepDefinitions-->>CucumberRunner: assertion_result
Class diagram for centralized timeout settings and usageclassDiagram
class TimeoutSettingsModule {
+timeouts scenario
+timeouts serverStartup
+timeouts serverShutdown
+timeouts healthProbe
+timeouts healthProbeInterval
+timeouts uiInit
+timeouts uiCleanup
+getTimeout(key)
}
class TimeoutKey {
}
class AcceptanceApiHooksModule {
+setDefaultTimeout(timeout)
}
TimeoutSettingsModule --> TimeoutKey : defines
AcceptanceApiHooksModule --> TimeoutSettingsModule : uses_getTimeout
Class diagram for community create UI and acceptance helpersclassDiagram
class CommunityCreateProps {
+onSave(values)
}
class CommunityCreate {
+props onSave
+state formLoading
+onFinish(values)
}
class CommunityUiNotes {
+communityName string
+formSubmitted boolean
+lastValidationError string
}
class ReactRender {
+mountComponent(ui, options)
+unmountComponent()
+getRendered()
}
class CreateCommunityTask {
+setContainer(container)
+getContainer()
+flushAsync()
+CreateCommunity(name)
}
class UiCommunityPage {
+fillName(name)
+clickCreate()
+firstValidationError
+errorToast
}
class CommunityPage {
+fillName(name)
+clickCreate()
+firstValidationError
+errorToast
}
class JsdomPageAdapter {
+fill(selector, value)
+click(selector)
+getText(selector)
}
class JsdomElementHandle {
+fill(value)
+click()
+check()
}
class CommunityCreateStepDefinitions {
+Given_is_an_authenticated_community_owner(actorName)
+When_creates_a_community_with(actorName, dataTable)
+When_attempts_to_create_a_community_with(actorName, dataTable)
+Then_community_should_be_created_successfully()
+Then_community_name_should_be(expectedName)
+Then_should_see_a_community_error_for(actorName, fieldName)
+Then_no_community_should_be_created()
}
CommunityCreate ..> CommunityCreateProps : implements
CommunityCreateStepDefinitions --> CommunityCreate : mounts
CommunityCreateStepDefinitions --> ReactRender : uses_mountComponent
CommunityCreateStepDefinitions --> CreateCommunityTask : uses
CreateCommunityTask --> UiCommunityPage : drives
UiCommunityPage ..> CommunityPage : alias_of
UiCommunityPage --> JsdomPageAdapter : constructed_with
JsdomPageAdapter --> JsdomElementHandle : creates
CommunityCreateStepDefinitions --> CommunityUiNotes : stores_form_state
CreateCommunityTask --> CreateCommunityTask : uses_getContainer
CommunityCreate --> CommunityUiNotes : onSave_updates_via_notes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
… ignore due to type issues
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="build-pipeline/scripts/merge-coverage.js" line_range="55" />
<code_context>
+ if (entry.name !== 'node_modules' && entry.name !== '.git') {
+ findLcovFiles(fullPath);
+ }
+ } else if (entry.name === 'lcov.info' && fullPath.includes('/coverage/')) {
+ lcovFiles.push(fullPath);
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Path check for coverage directory is POSIX-only and will miss files on Windows
`fullPath.includes('/coverage/')` relies on POSIX path separators, so it won’t match on Windows where `fullPath` uses `\`. Normalize the path first (e.g. `const normalized = fullPath.replace(/\\/g, '/')`) before checking `includes`, or use `path.normalize`/`path.sep` to keep this cross‑platform.
</issue_to_address>
### Comment 2
<location path="packages/ocom-verification/acceptance-ui/src/contexts/community/tasks/create-community.ts" line_range="5" />
<code_context>
-import type { CommunityUiNotes } from '../abilities/community-types.ts';
+import { Interaction } from '@serenity-js/core';
+
+let currentContainer: HTMLElement | null = null;
+
+export function setContainer(container: HTMLElement): void {
</code_context>
<issue_to_address>
**issue (bug_risk):** Global mutable `currentContainer` risks cross-scenario leakage and race conditions
In parallel runs or when scenarios overlap, this shared variable can be overwritten between steps, causing flaky, hard-to-debug failures. Prefer storing the container on the scenario World/actor (e.g., via Serenity `notes` or a custom ability) so each scenario/actor keeps its own reference instead of using a singleton.
</issue_to_address>
### Comment 3
<location path="packages/ocom-verification/acceptance-ui/src/shared/support/ui/react-render.ts" line_range="9-11" />
<code_context>
+
+let rendered: RenderResult | null = null;
+
+export interface MountOptions {
+ mocks?: MockedResponse[];
+ wrapWithRouter?: boolean;
+}
+
</code_context>
<issue_to_address>
**suggestion:** `wrapWithRouter` option is defined but never used when mounting components
The `MountOptions` interface exposes `wrapWithRouter`, but `mountComponent` currently ignores it and always uses the same wrapper tree. To avoid confusing callers and hiding missing routing context in tests, either implement conditional router wrapping based on this flag or remove it from the options API.
Suggested implementation:
```typescript
let rendered: RenderResult | null = null;
```
```typescript
export interface MountOptions {
mocks?: MockedResponse[];
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This reverts commit d222c6b.
…taging for sonar coverage to pick up
Contributor
Author
…d oauth2 server, uneeded logic was added
…e log noise of an uneeded step
… based on such, remove debugging
nnoce14
previously approved these changes
May 18, 2026
… test vite server to enable this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary by Sourcery
Align verification test infrastructure, community UI flows, and coverage tooling around the ocom-verification packages and shared timeout/server abstractions.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: