-
Notifications
You must be signed in to change notification settings - Fork 0
Fix issues found in formal code reviews of all review-sets #125
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
f380dbe
Fix resource leaks and label substring matching found in formal reviews
Copilot 3ef7814
Fix .reviewmark.yaml: remove unit design docs and source files from s…
Copilot a20d59a
Add label regression tests and unit review-sets for all RepoConnector…
Copilot 61e3d5e
Fix lint failures: table alignment in new markdown docs and rephrase …
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
35 changes: 35 additions & 0 deletions
35
docs/design/build-mark/repo-connectors/mock-repo-connector.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # MockRepoConnector | ||
|
|
||
| ## Overview | ||
|
|
||
| `MockRepoConnector` is an in-memory implementation of `IRepoConnector` used for | ||
| self-validation and unit testing. It returns a fixed, deterministic dataset | ||
| without making any network or filesystem calls. | ||
|
|
||
| ## Data Model | ||
|
|
||
| The connector holds hard-coded mappings used to build the `BuildInformation` response: | ||
|
|
||
| | Field | Type | Description | | ||
| |----------------------|------------------------------------|----------------------------------------------| | ||
| | `_issueTitles` | `Dictionary<string, string>` | Issue ID -> title | | ||
| | `_issueTypes` | `Dictionary<string, string>` | Issue ID -> type (bug/feature/documentation) | | ||
| | `_pullRequestIssues` | `Dictionary<string, List<string>>` | PR ID -> linked issue IDs | | ||
| | `_tagHashes` | `Dictionary<string, string>` | Tag name -> commit hash | | ||
| | `_openIssues` | `List<string>` | IDs of issues that remain open | | ||
|
|
||
| ## Methods | ||
|
|
||
| ### `GetBuildInformationAsync(version?) → BuildInformation` | ||
|
|
||
| Resolves tag history and the current commit hash from the internal dictionaries, | ||
| determines the target and baseline versions, collects changes and known issues, | ||
| and returns a fully populated `BuildInformation` record. The logic mirrors the | ||
| production GitHubRepoConnector flow but operates entirely on in-memory data. | ||
|
|
||
| ## Interactions | ||
|
|
||
| | Unit / Subsystem | Role | | ||
| |---------------------|----------------------------------------------------------------| | ||
| | `RepoConnectorBase` | Base class providing `FindVersionIndex` and command delegation | | ||
| | `Validation` | Instantiates `MockRepoConnector` directly for self-tests | |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| # ProcessRunner | ||
|
|
||
| ## Overview | ||
|
|
||
| `ProcessRunner` is a static helper class that executes external shell commands and | ||
| captures their standard output. It provides two public methods: `RunAsync`, which | ||
| throws on failure, and `TryRunAsync`, which returns `null` on failure. | ||
|
|
||
| ## Methods | ||
|
|
||
| ### `RunAsync(command, arguments) → string` | ||
|
|
||
| Starts the specified process, captures stdout and stderr asynchronously, waits for | ||
| exit, and returns the trimmed stdout string. Throws `InvalidOperationException` if | ||
| the process exits with a non-zero exit code. | ||
|
|
||
| ### `TryRunAsync(command, arguments) → string?` | ||
|
|
||
| Executes the process and returns the stdout string if the exit code is zero, or | ||
| `null` if the process fails or throws any exception. This method never propagates | ||
| exceptions to its callers. | ||
|
|
||
| ## Interactions | ||
|
|
||
| | Unit / Subsystem | Role | | ||
| |------------------------|---------------------------------------------------------| | ||
| | `RepoConnectorBase` | Delegates `RunCommandAsync` to `ProcessRunner.RunAsync` | | ||
| | `RepoConnectorFactory` | Uses `TryRunAsync` to inspect the git remote URL | | ||
| | `GitHubRepoConnector` | Calls `RunCommandAsync` (via base) for git and gh CLI | |
35 changes: 35 additions & 0 deletions
35
docs/design/build-mark/repo-connectors/repo-connector-base.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # IRepoConnector and RepoConnectorBase | ||
|
|
||
| ## Overview | ||
|
|
||
| `IRepoConnector` defines the contract that all repository connectors must satisfy. | ||
| `RepoConnectorBase` is an abstract class implementing `IRepoConnector` and providing | ||
| shared utilities used by concrete connectors. | ||
|
|
||
| ## Interface | ||
|
|
||
| `IRepoConnector` exposes a single method: | ||
|
|
||
| | Member | Kind | Description | | ||
| |-------------------------------------|--------|----------------------------------| | ||
| | `GetBuildInformationAsync(version)` | Method | Fetch complete build information | | ||
|
|
||
| ## Base Class | ||
|
|
||
| `RepoConnectorBase` provides: | ||
|
|
||
| | Member | Kind | Description | | ||
| |------------------------------------------|-------------------|------------------------------------------------------| | ||
| | `RunCommandAsync(command, arguments)` | Protected virtual | Delegates shell commands to `ProcessRunner.RunAsync` | | ||
| | `FindVersionIndex(versions, normalized)` | Protected static | Locates a version by normalized version string | | ||
|
|
||
| The `RunCommandAsync` method is `virtual` so that test subclasses can override it | ||
| with mock implementations that return fixed strings without spawning real processes. | ||
|
|
||
| ## Interactions | ||
|
|
||
| | Unit / Subsystem | Role | | ||
| |-----------------------|------------------------------------------------------| | ||
| | `ProcessRunner` | Used by `RunCommandAsync` to execute shell commands | | ||
| | `GitHubRepoConnector` | Concrete implementation that inherits this base | | ||
| | `MockRepoConnector` | Test implementation that overrides `RunCommandAsync` | |
28 changes: 28 additions & 0 deletions
28
docs/design/build-mark/repo-connectors/repo-connector-factory.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # RepoConnectorFactory | ||
|
|
||
| ## Overview | ||
|
|
||
| `RepoConnectorFactory` is a static factory class that creates the appropriate | ||
| `IRepoConnector` implementation based on the runtime environment. | ||
|
|
||
| ## Methods | ||
|
|
||
| ### `Create() → IRepoConnector` | ||
|
|
||
| Returns a new `GitHubRepoConnector` if any of the following conditions are met: | ||
|
|
||
| 1. The `GITHUB_ACTIONS` environment variable is non-empty. | ||
| 2. The `GITHUB_WORKSPACE` environment variable is non-empty. | ||
| 3. The git remote URL (obtained by running `git remote get-url origin`) contains `github.com`. | ||
|
|
||
| If none of the above conditions are met the factory defaults to returning a new | ||
| `GitHubRepoConnector` (the only connector implementation currently available). | ||
|
|
||
| ## Interactions | ||
|
|
||
| | Unit / Subsystem | Role | | ||
| |-----------------------|-------------------------------------------------------------| | ||
| | `IRepoConnector` | Return type of `Create` | | ||
| | `GitHubRepoConnector` | The concrete connector returned by `Create` | | ||
| | `ProcessRunner` | Used via `TryRunAsync` to inspect the git remote URL | | ||
| | `Program` | Calls `RepoConnectorFactory.Create()` to obtain a connector | |
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
23 changes: 23 additions & 0 deletions
23
docs/reqstream/build-mark/repo-connectors/mock-repo-connector.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| --- | ||
| # Software Unit Requirements for the MockRepoConnector Class | ||
| # | ||
| # MockRepoConnector is an in-memory repository connector used for | ||
| # deterministic testing. It provides fixed build information covering | ||
| # multiple versions, change types, and known issues. | ||
|
|
||
| sections: | ||
| - title: MockRepoConnector Unit Requirements | ||
| requirements: | ||
| - id: BuildMark-MockRepoConnector-Deterministic | ||
| title: The MockRepoConnector class shall return deterministic build information for testing. | ||
| justification: | | ||
| Tests that exercise the report generation and self-validation logic must run | ||
| without external dependencies. The MockRepoConnector provides a fixed, | ||
| predictable dataset so tests can assert exact outcomes. | ||
| tests: | ||
| - MockRepoConnector_Constructor_CreatesInstance | ||
| - MockRepoConnector_ImplementsInterface | ||
| - MockRepoConnector_GetBuildInformationAsync_ReturnsExpectedVersion | ||
| - MockRepoConnector_GetBuildInformationAsync_ReturnsCompleteInformation | ||
| - MockRepoConnector_GetBuildInformationAsync_WithValidVersionFromTags_ReturnsCorrectBaseline | ||
| - MockRepoConnector_GetBuildInformationAsync_CategorizesChangesCorrectly |
23 changes: 23 additions & 0 deletions
23
docs/reqstream/build-mark/repo-connectors/process-runner.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| --- | ||
| # Software Unit Requirements for the ProcessRunner Class | ||
| # | ||
| # ProcessRunner provides static helpers for running external processes | ||
| # (such as git and gh CLI) and capturing their output. It exposes both | ||
| # a throwing variant (RunAsync) and a non-throwing variant (TryRunAsync). | ||
|
|
||
| sections: | ||
| - title: ProcessRunner Unit Requirements | ||
| requirements: | ||
| - id: BuildMark-ProcessRunner-RunAsync | ||
| title: The ProcessRunner class shall execute external commands and return their output. | ||
| justification: | | ||
| Repository connectors need to run git and gh CLI commands to discover the | ||
| repository URL, current branch, current commit hash, and authentication | ||
| tokens. A centralized process runner ensures consistent error handling and | ||
| output capture across all callers. | ||
| tests: | ||
| - ProcessRunner_TryRunAsync_WithValidCommand_ReturnsOutput | ||
| - ProcessRunner_TryRunAsync_WithInvalidCommand_ReturnsNull | ||
| - ProcessRunner_TryRunAsync_WithNonZeroExitCode_ReturnsNull | ||
| - ProcessRunner_RunAsync_WithValidCommand_ReturnsOutput | ||
| - ProcessRunner_RunAsync_WithFailingCommand_ThrowsException |
29 changes: 29 additions & 0 deletions
29
docs/reqstream/build-mark/repo-connectors/repo-connector-base.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| --- | ||
| # Software Unit Requirements for the IRepoConnector Interface and RepoConnectorBase Class | ||
| # | ||
| # IRepoConnector defines the contract for all repository connectors. | ||
| # RepoConnectorBase provides a common base implementation including process | ||
| # execution delegation and version-list search utilities. | ||
|
|
||
| sections: | ||
| - title: RepoConnectorBase Unit Requirements | ||
| requirements: | ||
| - id: BuildMark-RepoConnectorBase-Interface | ||
| title: The IRepoConnector interface shall define the contract for all repository connectors. | ||
| justification: | | ||
| A common interface ensures that all repository connectors are interchangeable, | ||
| enabling the factory and program to work with any connector implementation | ||
| without depending on concrete types. | ||
| tests: | ||
| - MockRepoConnector_ImplementsInterface | ||
| - GitHubRepoConnector_ImplementsInterface_ReturnsTrue | ||
|
|
||
| - id: BuildMark-RepoConnectorBase-CommandExecution | ||
| title: The RepoConnectorBase class shall provide a virtual command execution method for shell commands. | ||
| justification: | | ||
| Abstracting command execution behind a virtual method allows test subclasses | ||
| to override it with mock implementations, enabling deterministic unit tests | ||
| that do not spawn real processes. | ||
| tests: | ||
| - MockRepoConnector_Constructor_CreatesInstance | ||
| - GitHubRepoConnector_Constructor_CreatesInstance |
19 changes: 19 additions & 0 deletions
19
docs/reqstream/build-mark/repo-connectors/repo-connector-factory.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| --- | ||
| # Software Unit Requirements for the RepoConnectorFactory Class | ||
| # | ||
| # RepoConnectorFactory creates the appropriate IRepoConnector implementation | ||
| # based on the runtime environment (GitHub Actions environment variables, | ||
| # or git remote URL inspection). | ||
|
|
||
| sections: | ||
| - title: RepoConnectorFactory Unit Requirements | ||
| requirements: | ||
| - id: BuildMark-RepoConnectorFactory-Create | ||
| title: The RepoConnectorFactory class shall create the appropriate repository connector for the environment. | ||
| justification: | | ||
| The factory encapsulates connector selection logic so that the Program class | ||
| does not need to know which connector to use. The correct connector is chosen | ||
| automatically based on GitHub Actions environment variables or git remote URL. | ||
| tests: | ||
| - RepoConnectorFactory_Create_ReturnsConnector | ||
| - RepoConnectorFactory_Create_ReturnsGitHubConnectorForThisRepo |
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.