-
Notifications
You must be signed in to change notification settings - Fork 12
test: initial support for Verify and DotNetRDF + OSLC IResource #454
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds Verify-based snapshot testing: project/package updates, test Verify calls and a ModuleInitializer registering TTL converters for RDF graphs/resources, new .verified/.ttl snapshots, VCS attributes/ignore rules, and duplicated .editorconfig blocks for received/verified file patterns. Changes
Sequence Diagram(s)sequenceDiagram
participant Init as VerifyInit (ModuleInitializer)
participant Test as RdfXmlMediaTypeFormatterTests
participant Verify as Verify Library
participant Conv as TTL Converter
participant FS as Snapshots Storage
Note over Init,Verify: Test verifier initialization
Init->>Verify: Set snapshots dir "Snapshots"
Init->>Verify: Register ".ttl" extension
Init->>Conv: Register IGraph -> TTL converter
Init->>Conv: Register IResource -> TTL converter
Note over Test,Verify: Test execution
Test->>Test: Build ChangeRequest / RDF graph(s)
Test->>Verify: await Verify(changeRequest1)
Verify->>Conv: Convert IResource -> canonical TTL
Conv-->>Verify: TTL
Verify->>FS: Compare to `.verified.ttl`
FS-->>Verify: Match/Mismatch
Verify-->>Test: Assertion result
Test->>Verify: await Verify(rdfGraph)
Verify->>Conv: Convert IGraph -> canonical TTL
Conv-->>Verify: TTL
Verify->>FS: Compare to `.verified.ttl` (collection)
FS-->>Verify: Match/Mismatch
Verify-->>Test: Assertion result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #454 +/- ##
=======================================
Coverage 49.55% 49.55%
=======================================
Files 166 166
Lines 15388 15388
Branches 1024 1024
=======================================
Hits 7626 7626
Misses 7479 7479
Partials 283 283 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.editorconfig (1)
465-473: Snapshot formatting overrides are reasonableScoping relaxed formatting rules to
*.{received,verified}.{json,txt,xml}is a good way to prevent IDE/formatter noise on snapshot files, while keeping the rest of the codebase strict.If you ever add other snapshot formats (e.g.,
.ttl), consider extending this section or adding a similar one for those extensions.OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/OSLC4Net.Core.DotNetRdfProviderTests.csproj (1)
7-9: ProjectReference normalization is harmlessSwitching the project references onto single‑line Include entries is stylistic and doesn’t change behavior; it keeps the project file consistent and readable.
If you touch more references later, consider keeping a consistent ordering (e.g., alphabetic) to reduce diff noise.
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/Snapshots/RdfXmlMediaTypeFormatterTests.TestRdfXmlSerializationAsync.verified.ttl (1)
1-13: TTL snapshot content looks coherent with the ChangeRequest testThe triples (subject URI,
cm#affectedByDefect,cm#fixed,cm#severity, and the reified statement) match the semantics of theChangeRequestbeing serialized in the test, so this snapshot should give stable, readable coverage of the RDF output.If these vocabularies are reused in many snapshots, consider adding prefix comments or documentation somewhere to make it easier for contributors unfamiliar with the OSLC vocabulary to interpret the TTL.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.editorconfig(1 hunks).gitattributes(1 hunks).gitignore(1 hunks)OSLC4Net_SDK/Directory.Packages.props(2 hunks)OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/OSLC4Net.Core.DotNetRdfProviderTests.csproj(2 hunks)OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/RdfXmlMediaTypeFormatterTests.cs(1 hunks)OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/Snapshots/RdfXmlMediaTypeFormatterTests.TestRdfXmlSerializationAsync.verified.ttl(1 hunks)OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
⚙️ CodeRabbit configuration file
**/*.cs: Review the C# code against the Microsoft Framework design guidelines as well as 10 guidelines
from "Building Maintainable Software" by Joost Visser. Point out deviations and improvements.
Files:
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.csOSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/RdfXmlMediaTypeFormatterTests.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (csharp)
- GitHub Check: test (ubuntu-24.04, 10.x, net10.0)
- GitHub Check: test (windows-latest, 10.x, net10.0)
- GitHub Check: test (ubuntu-24.04-arm, 10.x, net10.0)
🔇 Additional comments (5)
.gitattributes (1)
1-4: Verified artifacts attributes look consistent with snapshot usageMarking verified text files as LF/UTF‑8 and verified binaries as binary is appropriate and should avoid cross‑platform churn in snapshot tests.
.gitignore (1)
3-6: Ignoring*.received.*is appropriate for Verify artifactsTreating
*.received.*as ignored output keeps the repo clean from transient Verify snapshot files while still allowing*.verified.*to be committed.OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/OSLC4Net.Core.DotNetRdfProviderTests.csproj (2)
4-4: Implicit usings are fine for this test projectEnabling
<ImplicitUsings>enable</ImplicitUsings>is consistent with modern .NET project style and supports the LINQ/Task usage seen in the tests without extra clutter.
20-20: Package reference correctly wired to central versionVerification confirms the setup is correct:
Verify.XunitV3is referenced without a version in the csproj file and the central version31.7.1is properly defined inDirectory.Packages.props. The package will be resolved correctly through central package management.OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.cs (1)
1-49: Verify imports are properly resolved; fix minor formatting on line 34The
VerifierandVerifierSettingstypes are correctly in scope through implicit usings from theVerify.XunitV3NuGet package, which is referenced in the project's.csprojwith<ImplicitUsings>enable</ImplicitUsings>configured. No additional using directives are needed.The one small fix: remove the extra leading space on line 34 before
var rdfc = new RdfCanonicalizer();.The
canonicalizedRdfDataset.OutputDataset.Graphs.Single()pattern is safe here since theGraphCollectionis initialized with exactly one graph; the early-fail design is intentional and appropriate.
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/RdfXmlMediaTypeFormatterTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.cs (2)
24-41: Consider validating graph count before calling Single().The
Single()call on line 35 will throw anInvalidOperationExceptionif the canonicalized dataset doesn't contain exactly one graph. Consider adding validation or error handling to provide a more informative error message if this assumption is violated.Apply this diff to add validation:
var canonicalizedRdfDataset = rdfc.Canonicalize(new TripleStore(graphCollection)); - var newGraph = canonicalizedRdfDataset.OutputDataset.Graphs.Single(); + var graphs = canonicalizedRdfDataset.OutputDataset.Graphs.ToList(); + if (graphs.Count != 1) + { + throw new InvalidOperationException($"Expected exactly one graph after canonicalization, but found {graphs.Count}"); + } + var newGraph = graphs[0];
24-62: Consider extracting duplicate canonicalization logic.The canonicalization and serialization logic is duplicated between the
IGraphandIResourceconverters (lines 28-39 and 48-60). Consider extracting this into a private helper method to follow the DRY principle from "Building Maintainable Software."Example refactoring:
private static string CanonicalizeAndSerialize(IGraph graph) { using var writer = new StringWriter(); var rdfc = new RdfCanonicalizer(); var graphCollection = new GraphCollection { { graph, false } }; var canonicalizedRdfDataset = rdfc.Canonicalize(new TripleStore(graphCollection)); var newGraph = canonicalizedRdfDataset.OutputDataset.Graphs.Single(); var ttlWriter = new VDS.RDF.Writing.CompressingTurtleWriter(); ttlWriter.Save(newGraph, writer); return writer.ToString(); }Then simplify the converters:
VerifierSettings.RegisterFileConverter<IGraph>((graph, context) => { var serialized = CanonicalizeAndSerialize(graph); return new(null, "ttl", serialized); }); VerifierSettings.RegisterFileConverter<IResource>((resource, context) => { var graph = DotNetRdfHelper.CreateDotNetRdfGraph([resource]); var serialized = CanonicalizeAndSerialize(graph); return new(null, "ttl", serialized); });OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/Snapshots/RdfXmlMediaTypeFormatterTests.TestRdfXmlCollectionSerializationAsync.verified.ttl (1)
1-3: Standard RDF prefixes are correctly defined.The prefix declarations for rdf, rdfs, and xsd are standard and properly formatted.
Consider adding OSLC namespace prefixes for better readability:
@prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>. @prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#>. @prefix xsd: <http://www.w3.org/2001/XMLSchema#>. +@prefix oslc: <http://open-services.net/ns/core#>. +@prefix oslc_cm: <http://open-services.net/ns/cm#>. +@prefix dcterms: <http://purl.org/dc/terms/>.This would make the URIs in lines 5-27 more concise (e.g.,
oslc_cm:affectedByDefectinstead of<http://open-services.net/ns/cm#affectedByDefect>).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/RdfXmlMediaTypeFormatterTests.cs(2 hunks)OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/Snapshots/RdfXmlMediaTypeFormatterTests.TestRdfXmlCollectionSerializationAsync.verified.ttl(1 hunks)OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/RdfXmlMediaTypeFormatterTests.cs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
⚙️ CodeRabbit configuration file
**/*.cs: Review the C# code against the Microsoft Framework design guidelines as well as 10 guidelines
from "Building Maintainable Software" by Joost Visser. Point out deviations and improvements.
Files:
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.cs
🪛 GitHub Check: Format (dotnet format)
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.cs
[warning] 16-16:
Avoid multiple blank lines
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (windows-latest, 10.x, net10.0)
- GitHub Check: test (ubuntu-24.04, 10.x, net10.0)
- GitHub Check: test (ubuntu-24.04-arm, 10.x, net10.0)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (2)
OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/Snapshots/RdfXmlMediaTypeFormatterTests.TestRdfXmlCollectionSerializationAsync.verified.ttl (2)
5-14: Reified statements correctly represent bidirectional relationships.The blank nodes represent reified RDF statements that mirror the direct
affectedByDefecttriples in lines 15 and 19. This appears intentional for testing the serializer's handling of reified statements and relationship metadata.
15-22: ChangeRequest resources are well-formed and consistent.The two ChangeRequest resources correctly represent:
- Bidirectional
affectedByDefectrelationships (consistent with the reified statements)- Distinct
fixedboolean values (true/false)- Appropriate OSLC CM properties and types
| <http://com/somewhere/changerequests> rdfs:member <http://com/somewhere/changeRequest1>, | ||
| <http://com/somewhere/changeRequest2>. | ||
| <http://com/somewhere/changerequests?page=20> <http://open-services.net/ns/core#nextPage> <http://com/somewhere/changerequests?page=21>; | ||
| <http://open-services.net/ns/core#totalCount> "2"; | ||
| a <http://open-services.net/ns/core#ResponseInfo>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify totalCount data type against OSLC specification.
The collection and pagination structure are correct, but line 26 has totalCount as an untyped string literal "2" rather than a typed integer. According to OSLC Core specifications, totalCount should typically be an xsd:integer.
Please verify whether the serializer is correctly outputting type information for numeric properties. Check the OSLC Core specification for the expected data type of oslc:totalCount.
If the spec requires xsd:integer, the expected output should be:
-<http://com/somewhere/changerequests?page=20> <http://open-services.net/ns/core#nextPage> <http://com/somewhere/changerequests?page=21>;
- <http://open-services.net/ns/core#totalCount> "2";
- a <http://open-services.net/ns/core#ResponseInfo>.
+<http://com/somewhere/changerequests?page=20> <http://open-services.net/ns/core#nextPage> <http://com/somewhere/changerequests?page=21>;
+ <http://open-services.net/ns/core#totalCount> "2"^^xsd:integer;
+ a <http://open-services.net/ns/core#ResponseInfo>.🌐 Web query:
OSLC Core specification totalCount data type xsd:integer
💡 Result:
Yes — OSLC Core defines oslc:totalCount as an integer (zero-or-one, optional). Its value SHOULD be non‑negative and, for query responses, SHOULD represent the total number of matching results. [1][2]
Sources:
- OSLC Core — Vocabulary (totalCount = integer; SHOULD be non‑negative). [1]
- OSLC Core — ResponseInfo shape (oslc:totalCount value-type: integer; occurrence: zero-or-one). [2]
Fix totalCount serialization to include xsd:integer type annotation.
Line 26 has totalCount as an untyped string literal "2", but OSLC Core defines oslc:totalCount as an integer (zero-or-one, optional). The RDF serializer should output the value with explicit type annotation:
-<http://com/somewhere/changerequests?page=20> <http://open-services.net/ns/core#nextPage> <http://com/somewhere/changerequests?page=21>;
- <http://open-services.net/ns/core#totalCount> "2";
+<http://com/somewhere/changerequests?page=20> <http://open-services.net/ns/core#nextPage> <http://com/somewhere/changerequests?page=21>;
+ <http://open-services.net/ns/core#totalCount> "2"^^xsd:integer;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <http://com/somewhere/changerequests> rdfs:member <http://com/somewhere/changeRequest1>, | |
| <http://com/somewhere/changeRequest2>. | |
| <http://com/somewhere/changerequests?page=20> <http://open-services.net/ns/core#nextPage> <http://com/somewhere/changerequests?page=21>; | |
| <http://open-services.net/ns/core#totalCount> "2"; | |
| a <http://open-services.net/ns/core#ResponseInfo>. | |
| <http://com/somewhere/changerequests> rdfs:member <http://com/somewhere/changeRequest1>, | |
| <http://com/somewhere/changeRequest2>. | |
| <http://com/somewhere/changerequests?page=20> <http://open-services.net/ns/core#nextPage> <http://com/somewhere/changerequests?page=21>; | |
| <http://open-services.net/ns/core#totalCount> "2"^^xsd:integer; | |
| a <http://open-services.net/ns/core#ResponseInfo>. |
| // Verifier.UseProjectRelativeDirectory("Snapshots"); | ||
| Verifier.UseSourceFileRelativeDirectory("Snapshots"); | ||
|
|
||
|
|
||
| // VerifierSettings.AddExtraSettings( | ||
| // _ => _.TypeNameHandling = TypeNameHandling.All); | ||
| // VerifierSettings.AddScrubber(_ => _.Replace("String to verify", "new value")); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented code and extra blank lines.
The commented code (lines 14, 18-20) should either be removed or documented with a TODO/NOTE explaining why it's retained. Additionally, the multiple blank lines (16-17) flagged by static analysis should be reduced to a single blank line.
Apply this diff to clean up the code:
- // Verifier.UseProjectRelativeDirectory("Snapshots");
Verifier.UseSourceFileRelativeDirectory("Snapshots");
-
- // VerifierSettings.AddExtraSettings(
- // _ => _.TypeNameHandling = TypeNameHandling.All);
- // VerifierSettings.AddScrubber(_ => _.Replace("String to verify", "new value"));
-
FileExtensions.AddTextExtension("ttl");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Verifier.UseProjectRelativeDirectory("Snapshots"); | |
| Verifier.UseSourceFileRelativeDirectory("Snapshots"); | |
| // VerifierSettings.AddExtraSettings( | |
| // _ => _.TypeNameHandling = TypeNameHandling.All); | |
| // VerifierSettings.AddScrubber(_ => _.Replace("String to verify", "new value")); | |
| Verifier.UseSourceFileRelativeDirectory("Snapshots"); | |
| FileExtensions.AddTextExtension("ttl"); |
🧰 Tools
🪛 GitHub Check: Format (dotnet format)
[warning] 16-16:
Avoid multiple blank lines
🤖 Prompt for AI Agents
In OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.cs around
lines 14 to 21, remove the commented-out Verifier.UseProjectRelativeDirectory
line and the commented VerifierSettings/AddScrubber block (or replace them with
a single TODO/NOTE comment if they must be retained), and collapse the multiple
blank lines into a single blank line so there is only one empty line between
statements; ensure no lingering commented code remains unless explicitly
documented with a concise TODO/NOTE explaining why it is kept.
|
|
||
| // https://www.w3.org/TR/rdf-canon/ prescribes N-Quads serialization, but let's try Turtle for better readability | ||
| var ttlWriter = new VDS.RDF.Writing.CompressingTurtleWriter(); | ||
| // ttlWriter.Save(graph, writer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented out code.
Line 59 contains commented code that should be removed to improve code clarity.
Apply this diff:
var ttlWriter = new VDS.RDF.Writing.CompressingTurtleWriter();
- // ttlWriter.Save(graph, writer);
ttlWriter.Save(newGraph, writer);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // ttlWriter.Save(graph, writer); | |
| var ttlWriter = new VDS.RDF.Writing.CompressingTurtleWriter(); | |
| ttlWriter.Save(newGraph, writer); |
🤖 Prompt for AI Agents
In OSLC4Net_SDK/Tests/OSLC4Net.Core.DotNetRdfProviderTests/VerifyInit.cs around
line 59, remove the commented-out line "// ttlWriter.Save(graph, writer);" to
clean up dead code; simply delete that commented line so the file contains only
active, relevant code and no leftover commented statements.
Summary by CodeRabbit
Tests
Chores