Skip to content

Document parallel test execution and pure function requirements in settest/graphtest#58

Draft
Copilot wants to merge 4 commits intomainfrom
copilot/finish-remaining-todos-issue-36
Draft

Document parallel test execution and pure function requirements in settest/graphtest#58
Copilot wants to merge 4 commits intomainfrom
copilot/finish-remaining-todos-issue-36

Conversation

Copy link
Contributor

Copilot AI commented Nov 2, 2025

Completes the final TODO from issue #36: documenting that test suite functions run in parallel and require pure builder functions.

Changes

set/settest/tester.go

  • Added docs to TestReadOnly and TestMutable specifying that sliceToSet must be pure (no side effects, deterministic)
  • Documented element type (int) and parameter contracts to match graphtest style
  • Added backticks around parameter references for consistency

graph/graphtest/tester.go

  • Added docs to TestReadOnly and TestMutable specifying that emptyGraph must be pure
  • Added backticks around parameter references for consistency
  • Note: addNode and putEdge parameters are not required to be pure functions, as they are designed to allow users to specify how nodes and edges are added regardless of graph mutability

All test functions internally use t.Parallel(), so builder functions (emptyGraph and sliceToSet) passed by callers must avoid shared mutable state to prevent race conditions.

Example usage constraint now documented:

// ❌ Non-pure: reuses mutable state across parallel tests
var globalCache = make(map[int]Set[int])
settest.TestMutable(t, func(elements []int) settest.MutableSet[int] {
    if s, ok := globalCache[len(elements)]; ok { return s } // Race!
    return set.Of(elements...)
})

// ✅ Pure: returns fresh instance every call
settest.TestMutable(t, func(elements []int) settest.MutableSet[int] {
    return set.Of(elements...)
})
Original prompt

Finish all remaining TODOs in GitHub issue #36.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Finish remaining TODOs in GitHub issue 36 Document parallel test execution and pure function requirements in settest/graphtest Nov 2, 2025
Copilot AI requested a review from jbduncan November 2, 2025 21:13
@jbduncan
Copy link
Owner

jbduncan commented Nov 2, 2025

@copilot Parameters addNode and putEdge are perfectly fine to be side-effecting functions because only emptyGraph is the builder function and addNode and putEdge are designed to allow users to specify how nodes and edges are added to their graph type, regardless if the graph is a mutable type or an immutable type. So please remove addNode and putEdge from the comment about running tests in parallel.

Copy link
Contributor Author

Copilot AI commented Nov 2, 2025

@copilot Parameters addNode and putEdge are perfectly fine to be side-effecting functions because only emptyGraph is the builder function and addNode and putEdge are designed to allow users ...

Fixed in commit 52489a5. Removed addNode and putEdge from the pure function requirement in TestReadOnly documentation. Only emptyGraph is now documented as needing to be pure. Also added backticks around all parameter references.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants