Skip to content

Conversation

DoctorKrolic
Copy link
Contributor

Fixes: #73909

Remade tests for semantic snippets from scratch:

  • Now semantic snippets are tested independently from completions. There were a few completion-related tests, moved them to SemanticSnippetCompletionProviderTests. If we need to test that integration point more in the future, we can add new tests to this class
  • As a result of decoupling new tests moved from EditorFeatures to Features layer
  • We can now also verify placeholders. They are specified in the markup as annotated spans {|0:placeholder|}, {|1:placeholder2|} etc.
  • Instead of workspace xml, which is hard to read, we now pass editorconfig and reference assemblies separately (I cannot think of a scenario where semantic snippets can need more advanced xml features, such as multiple documents and so on)
  • Some tests got simplified (e.g. for/forr test instead of testing Length and Count property separately now use combinatorial data), some got renamed for consistency accross all semantic snippet tests
  • Reused some common test data instead of duplicating [InlineData] blocks
  • Due to Semantic snippets test flaw #73909 there was a bug, which sneaked in unnoticed. Was a trivial fix, so included it into this PR

@DoctorKrolic DoctorKrolic requested a review from a team as a code owner August 19, 2024 18:34
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 19, 2024
@dotnet-policy-service dotnet-policy-service bot added Community The pull request was submitted by a contributor who is not a Microsoft employee. VSCode labels Aug 19, 2024
@DoctorKrolic
Copy link
Contributor Author

@akhera99 PTAL

Copy link
Member

@akhera99 akhera99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good, thank you for your continued contributions in this space!

@DoctorKrolic
Copy link
Contributor Author

Someone restart failed pipelines pls

@akhera99 akhera99 enabled auto-merge (squash) August 20, 2024 19:01
@akhera99 akhera99 merged commit 5e6bb7a into dotnet:main Aug 20, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Aug 20, 2024
@DoctorKrolic DoctorKrolic deleted the semantic-snippets-tests branch August 21, 2024 04:18
@dibarbet dibarbet modified the milestones: Next, 17.12 P2 Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Semantic snippets test flaw
3 participants