Skip to content
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

add Expect.because to document the reason for an assertion #660

Closed
13 of 14 tasks
robstoll opened this issue Oct 20, 2020 · 1 comment · Fixed by #729
Closed
13 of 14 tasks

add Expect.because to document the reason for an assertion #660

robstoll opened this issue Oct 20, 2020 · 1 comment · Fixed by #729
Assignees
Labels
Milestone

Comments

@robstoll
Copy link
Owner

robstoll commented Oct 20, 2020

Platform (all, jvm, js, android): all
Extension (none, kotlin 1.3): none

Code related feature

See robstoll/atrium-roadmap#15 for the full background.

We want to be able to state the reason for an assertion (could also be additional context to explain the assertion etc.).

expect(proposeAFileName())
  .because("? is not allowed in file names on Windows") {
    containsNot("?")
  }

And it should look like the following in reporting:

expect: "file?Name"     (String <11>)
◆ contains not: "?"    (String <12>) 
ℹ because: ? is not allowed in file names on Windows

Following the things you need to do:

core

  • introduce a new ExplanatoryAssertionGroupType named InformationAssertionGroupType
  • add withInformationType to ExplanatoryGroup.GroupTypeOption

core-robstoll-lib

  • add informationBulletPoint to TextExplanatoryAssertionGroupFormatter, use "ℹ ", and adjust formatGroupHeaderAndGetChildParameterObject accordingly

specs

  • adjust AsciiBulletPointReporterFactory, also add InformationAssertionGroupType and use (i) as ascii bullet point
  • add InformationAssertionGroupType to AssertionFormatterControllerSpec (see WarningAssertionGroupType)
  • copy TextWarningAssertionGroupFormatterSpec and replace Warning with Information
    • extend it in core-robstoll-lib

logic

  • extend AnyAssertions with a function because which takes a string and an assertionCreator-lambda (see other assertion functions in anyAssertions which take an assertionCreator-lambda)
  • implement because in DefaultAnyAssertions.kt by using the assertionBuilder and create an invisibleAssertionGroup where it consists of two assertions, first the collected assertions and second an ExplanatoryAssertionGroup with your new type containing only an ExplanatoryAssertion with the corresponding text, prefixed with "because " where because should be translated (use "weil " for german) -> i.e. u need to use a TranslatableWithArgs (search for existing code)
  • run ./gradlew generateLogic this will re-generate any.kt under src/generated

api-fluent

  • provide fun because in anyAssertions.kt and delegate to logic (see other functions in anyAssertion)
  • add @since 0.14.0 (adapt to current milestone) to KDOC
  • add because to AnyAssertionsSpec in specs-common and check that it works as expected:
    • because is shown if first assertion in the assertionCreator-lambda fails
    • because is shown if second assertion in the assertionCreator-lambda fails

api-infix

  • adapt AnyAssertionsSpec in specs-common, since we don't have an implementation yet, you can use "because not yet implemented in this API" to Companion:because and create in the companion object a method named because which delegates to logic

Bonus:

  • also implement a function for infix. This is a bit more advanced. In the end we want to be able to write expect(...) because of("? is not allowed in file names on Windows") { ... }
    • checkout listAssertions.kt and IndexWithCreator + fun index to see how you can achieve an infix because of -> introduce OfWithCreator

Your first contribution?

  • Write a comment I'll work on this if you would like to take this issue over.
    This way we get the chance to revise the description in case things have changed in the meantime, we might give you additional hints and we can assign the task to you, so that others do not start as well.
  • See Your first code contribution for guidelines.
  • Do not hesitate to ask questions here or to contact us via Atrium's slack channel if you need help
    (Invite yourself in case you do not have an account yet).
@Valefant
Copy link
Collaborator

@robstoll I'll take this one, if it's ok

@Valefant Valefant self-assigned this Oct 22, 2020
Valefant pushed a commit to Valefant/atrium that referenced this issue Oct 24, 2020
Valefant pushed a commit to Valefant/atrium that referenced this issue Oct 27, 2020
…toll#660-add-except-because

� Conflicts:
�	apis/fluent-en_GB/atrium-api-fluent-en_GB-common/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/samples/AnyAssertionSamples.kt
Valefant pushed a commit to Valefant/atrium that referenced this issue Oct 28, 2020
@robstoll robstoll added this to the 0.15.0 milestone Nov 12, 2020
@robstoll robstoll modified the milestones: 0.15.0, 0.16.0 Dec 10, 2020
robstoll pushed a commit that referenced this issue Dec 21, 2020
* add because method for documenting the reason for the following assertion/s

Co-authored-by: Valefant <valentino.bernardo_ciddio@smail.th-koeln.de>

The format in this commit is not yet correct
@robstoll robstoll modified the milestones: 0.16.0, 0.15.0 Dec 21, 2020
robstoll pushed a commit that referenced this issue Dec 21, 2020
* add because method for documenting the reason for the following assertion/s

Co-authored-by: Valefant <valentino.bernardo_ciddio@smail.th-koeln.de>

The format in this commit is not yet correct
robstoll pushed a commit that referenced this issue Dec 21, 2020
* add because method for documenting the reason for the following assertion/s

Co-authored-by: Valefant <valentino.bernardo_ciddio@smail.th-koeln.de>

The format in this commit is not yet correct
robstoll pushed a commit that referenced this issue Dec 23, 2020
* add because method for documenting the reason for the following assertion/s

Co-authored-by: Valefant <valentino.bernardo_ciddio@smail.th-koeln.de>

The format in this commit is not yet correct
robstoll added a commit that referenced this issue Dec 23, 2020
@robstoll robstoll linked a pull request Dec 23, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants