8000 Correct Qodana `failThreshold` by IT-VBFK · Pull Request #2437 · fluentassertions/fluentassertions · GitHub
[go: up one dir, main page]
Skip to content

Correct Qodana failThreshold#2437

Merged
jnyrup merged 1 commit intofluentassertions:developfrom
IT-VBFK:fix/qodana-issues
Nov 18, 2023
Merged

Correct Qodana failThreshold#2437
jnyrup merged 1 commit intofluentassertions:developfrom
IT-VBFK:fix/qodana-issues

Conversation

@IT-VBFK
Copy link
Contributor
@IT-VBFK IT-VBFK commented Nov 4, 2023 •
edited
Loading

Actually I don't know if protected would satisfy the qodana rule..

Let's try...

IMPORTANT

  • If the PR touches the public API, the changes have been approved in a separate issue with the "api-approved" label.
  • The code complies with the Coding Guidelines for C#.
  • The changes are covered by unit tests which follow the Arrange-Act-Assert syntax and the naming conventions such as is used in these tests.
  • If the PR adds a feature or fixes a bug, please update the release notes with a functional description that explains what the change means to consumers of this library, which are published on the website.
  • If the PR changes the public API the changes needs to be included by running AcceptApiChanges.ps1 or AcceptApiChanges.sh.
  • If the PR affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
    • Please also run ./build.sh --target spellcheck or .\build.ps1 --target spellcheck before pushing and check the good outcome

@github-actions
Copy link
github-actions bot commented Nov 4, 2023 •
edited
Loading

Qodana for .NET

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked

View the detailed Qodana report

To be able to view the detailed Qodana report, you can either:

  1. Register at Qodana Cloud and configure the action
  2. Use GitHub Code Scanning with Qodana
  3. Host Qodana report at GitHub Pages
  4. Inspect and use qodana.sarif.json (see the Qodana SARIF format for details)

To get *.log files or any other Qodana artifacts, run the action with upload-result option set to true,
so that the action will upload the files as the job artifacts:

      - name: 'Qodana Scan'
        uses: JetBrains/qodana-action@v2023.2.8
        with:
          upload-result: true
Contact Qodana team

Contact us at qodana-support@jetbrains.com

IT-VBFK
IT-VBFK commented Nov 4, 2023
View reviewed changes
qodana.yaml Outdated

linter: jetbrains/qodana-dotnet:latest
failThreshold: 13
failThreshold: 15
Copy link
Contributor Author
@IT-VBFK IT-VBFK Nov 4, 2023

Choose a reason for hiding this comment

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

This is going up, because we have a few errors for .netstandard2.0 and .netstandard2.1.

@coveralls
Copy link
coveralls commented Nov 4, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 6892930586

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.457%
Totals Coverage Status
Change from base Build 6889406160: 0.0%
Covered Lines: 11743
Relevant Lines: 11926
💛 - Coveralls

dennisdoomen
dennisdoomen requested changes Nov 8, 2023
View reviewed changes
@IT-VBFK IT-VBFK force-pushed the fix/qodana-issues branch 2 times, most recently from 03c1a19 to 00bb279 Compare November 11, 2023 16:15
@IT-VBFK IT-VBFK requested a review from dennisdoomen November 11, 2023 16:20
@jnyrup jnyrup added the testing Testing of Fluent Assertion itself label Nov 11, 2023
@IT-VBFK IT-VBFK force-pushed the fix/qodana-issues branch 3 times, most recently from 1b79363 to a442268 Compare November 11, 2023 16:44
dennisdoomen
dennisdoomen requested changes Nov 12, 2023
View reviewed changes
@IT-VBFK IT-VBFK force-pushed the fix/qodana-issues branch 4 times, most recently from 9bac152 to 43ceafe Compare November 13, 2023 17:33
@ITaluone
Copy link
Contributor
ITaluone commented Nov 15, 2023 •
edited
Loading

The tests in this PR are not necessary anymore, since #2457 is merged, which does exactly that: Using several EquivalencyOptions in this test:

fluentassertions/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs

Lines 12 to 65 in 8dca977

8000
public void Public_methods_follow_fluent_syntax()
{
// Arrange
var subject = new Root();
var expected = new RootDto();
// Act / Assert
subject.Should().BeEquivalentTo(expected,
options => options
.AllowingInfiniteRecursion()
.ComparingByMembers(typeof(Root))
.ComparingByMembers<RootDto>()
.ComparingByValue(typeof(Customer))
.ComparingByValue<CustomerDto>()
.ComparingEnumsByName()
.ComparingEnumsByValue()
.ComparingRecordsByMembers()
.ComparingRecordsByValue()
.Excluding(r => r.Level)
.ExcludingFields()
.ExcludingMissingMembers()
.ExcludingNestedObjects()
.ExcludingNonBrowsableMembers()
.ExcludingProperties()
.IgnoringCyclicReferences()
.IgnoringNonBrowsableMembersOnSubject()
.Including(r => r.Level)
.IncludingAllDeclaredProperties()
.IncludingAllRuntimeProperties()
.IncludingFields()
.IncludingInternalFields()
.IncludingInternalProperties()
.IncludingNestedObjects()
.IncludingProperties()
.RespectingDeclaredTypes()
.RespectingRuntimeTypes()
.ThrowingOnMissingMembers()
.Using(new ExtensibilitySpecs.DoEquivalencyStep(() => { }))
.Using(new MustMatchByNameRule())
.Using(new AllFieldsSelectionRule())
.Using(new ByteArrayOrderingRule())
.Using(StringComparer.OrdinalIgnoreCase)
.WithAutoConversion()
.WithAutoConversionFor(_ => false)
.WithoutAutoConversionFor(_ => true)
.WithoutMatchingRules()
.WithoutSelectionRules()
.WithoutStrictOrdering()
.WithoutStrictOrderingFor(r => r.Level)
.WithStrictOrdering()
.WithStrictOrderingFor(r => r.Level)
.WithTracing()
);
}

But the failThreshold does..

@IT-VBFK
Copy link
Contributor Author
IT-VBFK commented Nov 15, 2023 •
edited
Loading

@jnyrup @dennisdoomen What do you think? Are the tests from this PR obsolete?

IMHO they are, but it's up to you keeping or removing them.

@dennisdoomen
Copy link
Member
dennisdoomen commented Nov 15, 2023

What do you think? Are the tests from this PR obsolete?

According to the coverage report, they don't seem to add value.

@eNeRGy164
Copy link
Contributor
eNeRGy164 commented Nov 15, 2023

Not only branch coverage can be a reason for a test, mutation coverage can also be a factor.

@ITaluone
Copy link
Contributor
ITaluone commented Nov 16, 2023

Not only branch coverage can be a reason for a test, mutation coverage can also be a factor.

Note, that the whole reason for this PR was to silence Qodana.

@IT-VBFK
Copy link
Contributor Author
IT-VBFK commented Nov 16, 2023

What do you think? Are the tests from this PR obsolete?

According to the coverage report, they don't seem to add value.

Ok, I will remove them

@IT-VBFK IT-VBFK changed the title Fix qodana issues which were introduced in prior PRs Correct Qodana failThreshold Nov 16, 2023
@jnyrup jnyrup merged commit 2eba66d into fluentassertions:develop Nov 18, 2023
@IT-VBFK IT-VBFK deleted the fix/qodana-issues branch November 18, 2023 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dennisdoomen dennisdoomen dennisdoomen approved these changes

@jnyrup jnyrup Awaiting requested review from jnyrup

Assignees

No one assigned

Labels

testing Testing of Fluent Assertion itself

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@IT-VBFK @coveralls @ITaluone @dennisdoomen @eNeRGy164 @jnyrup
0