Skip to content

Increase code coverage #24

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

Increase code coverage #24

wants to merge 26 commits into from

Conversation

nilay-kapadia
Copy link

Increase low code coverage of the repository by:

  • Adding new test cases
  • Removing dead code
  • Adding new models to cover additional code

Closes #5

@nilay-kapadia nilay-kapadia added type:refactor Refactoring or improving of existing code version:minor labels Mar 2, 2023
@nilay-kapadia nilay-kapadia self-assigned this Mar 2, 2023
jarz
jarz reviewed Mar 2, 2023
View reviewed changes
@jarz
Copy link
Member

jarz commented Mar 2, 2023

These four files need to be checked-in again with the correct line endings, when I run dotnet test locally they're modified.

  • test/Generator.Tests.Generated/Address.cs
  • test/Generator.Tests.Generated/AddressTestMap.cs
  • test/Generator.Tests.Generated/TestModel.cs
  • test/Generator.Tests.Generated/TestModelData.cs

jarz
jarz suggested changes Mar 3, 2023
View reviewed changes
@jarz
Copy link
Member

jarz commented Mar 3, 2023

We should also bump this number up: https://github.com/microsoft/dtdl-model-generator/blob/2fc93de6ed017d531945d5231f9eca5027481853/.github/workflows/build.yml#LL21C6-L21C28

@jarz jarz requested a review from a team as a code owner March 3, 2023 21:57
@jarz
Copy link
Member

jarz commented Mar 7, 2023

I had to use WSL to figure out the problem:

"Could not find a part of the path '/home/tim/dtdl-model-generator/test/Generator.Tests/bin/Debug/net6/TestDtdlErrorModels\\TestDtdlErrorUnsupportedContentType'."

Since Linux uses / (and the Build action runs on Linux) we just had to adjust the directory separator. Luckily, Windows can also treat / as a directory separator too.

@jarz jarz mentioned this pull request Mar 17, 2023
Merged
osamadawoood
osamadawoood previously approved these changes Apr 3, 2023
View reviewed changes
@nilay-kapadia nilay-kapadia reopened this Apr 4, 2023
@nilay-kapadia nilay-kapadia requested a review from jarz April 4, 2023 21:26
jarz
jarz reviewed Apr 6, 2023
View reviewed changes
jarz
jarz suggested changes Apr 12, 2023
View reviewed changes
Copy link
Member

@jarz jarz left a comment

Choose a reason for hiding this comment

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

One final thing: can this line be updated to add CS0612 to the list? That will stop the obsolete warnings that are noise.

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

@jarz jarz jarz requested changes

@osamadawoood osamadawoood osamadawoood approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@nilay-kapadia nilay-kapadia

Labels
type:refactor Refactoring or improving of existing code version:patch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Increase code coverage to 100%
3 participants
@nilay-kapadia @jarz @osamadawoood