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

feature extractor for Iterator.next #1816

Merged
merged 19 commits into from
Aug 21, 2024

Conversation

hubtanaka
Copy link
Collaborator

@hubtanaka hubtanaka commented Aug 18, 2024

Issue #1765

logic

  • extend IteratorAssertions with a function next (see ListAssertions.get as a guideline)
  • implement next in DefaultIteratorAssertions.kt by using container.extractFeature... (see DefaultListAssertions.get)

api-fluent

  • provide a val which returns Expect in iteratorFeatureExtractors.kt (see listFeatureExtractors.kt as a guideline)
  • provide a fun which expects an assertionCreator-lambda and returns Expect (where E is the element type in Iterator) in iteratorFeatureExtractors.kt (see listFeatureExtractors.kt as a guideline)
  • extend or write a separate Spec named IteratorFeatureExtractorSpec in specs -> commonMain (see for instance ListExpectationsSpec) and extend it in atrium-api-fluent -> commonTest
  • add samples to IteratorFeatureExtractorSamples.kt (see ListFeatureExtractorSamples.kt as guideline -- try to provide the reason why an expectation fails)
  • add @sample with link to your sample method to the two functions in iteratorExpectations.kt
  • add @SInCE 1.3.0 (adapt to current milestone) to KDOC of the two functions in iteratorExpectations.kt

api-infix

  • provide a val which returns Expect in iteratorExpectations.kt (see listExpectations.kt as a guideline)
  • provide a fun which expects an assertionCreator-lambda and returns Expect (where E is the element type in Iterator) in iteratorFeatureExtractors.kt (see listFeatureExtractors.kt as a guideline)
  • extend or write a separate Spec named IteratorFeatureExtractorSpec in specs -> commonMain (see for instance ListExpectationsSpec) and extend it in atrium-api-infix -> commonTest
  • add samples to IteratorFeatureExtractorSamples.kt (see ListFeatureExtractorSamples.kt as guideline -- try to provide the reason why an expectation fails)
  • add @sample with link to your sample method to the two functions in iteratorExpectations.kt
  • add @SInCE 1.3.0 (adapt to current milestone) to KDOC of the two functions in iteratorExpectations.kt

I confirm that I have read the Contributor Agreements v1.0, agree to be bound on them and confirm that my contribution is compliant.

@hubtanaka hubtanaka changed the title Draft: feature extractor for Iterator.next feature extractor for Iterator.next Aug 18, 2024
Comment on lines 37 to 40
expect(iterator)
.next() // subject is now of type Int (actually 1)
.toBeGreaterThan(0) // subject is still of type Int (still 1)
.toBeLessThan(2)
Copy link
Owner

Choose a reason for hiding this comment

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

hm... I would remove // subject is still of type Int (still 1) because toBeGreaterThan does not touch the subject at all (in the contrast to next) -- I know, you copied this from ListFeatureExtractorSamples. Could you remove it there as well please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I'll remove the comments from 2 files.

@@ -30,4 +29,60 @@ class IteratorExpectationSamples {
iterator.next() // returns the next element in iteration
expect(iterator).notToHaveNext() // does not fail as by now iterator has no next element
}

@Test
fun nextFeature() {
Copy link
Owner

Choose a reason for hiding this comment

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

please move your sample functions to IteratorFeatureExtractorSamples.kt (this file does not yet exist you need to create it) and adjust the link in the KDoc in the api accordingly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. I'll do.

*
* @return The newly created [Expect] for the next element.
*
* @sample TODO
Copy link
Owner

Choose a reason for hiding this comment

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

just as reminder

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thx. added a link (file is changed though)

*
* @since 1.3.0
*/
fun <E, T : Iterator<E>> Expect<T>.next(assertionCreator: Expect<E>.() -> Unit): Expect<T> =
Copy link
Owner

Choose a reason for hiding this comment

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

move to iteratorFeatureExtractors.kt (the file does not exist yet, create it please)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

created a file an moved this fun there

*
* @since 1.3.0
*/
infix fun <E, T : Iterator<E>> Expect<T>.next(@Suppress("UNUSED_PARAMETER") next: next): Expect<E> =
Copy link
Owner

Choose a reason for hiding this comment

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

move to iteratorFeatureExtractors.kt (the file does not exist yet, create it please).

Please use o as filler object (see other infix function using o)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replaced next with o (file is changed though)

@robstoll
Copy link
Owner

robstoll commented Aug 20, 2024

@hubtanaka Atrium uses bc-validator for a try. As you change the api (new methods) we need to dump the api. Please execute the following (I guess the build will then pass):

./gradlew apiDump
KOTLIN_VERSION=1.9 ./gradlew apiDump

Thanks

Copy link

codecov bot commented Aug 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.83%. Comparing base (7ca7fa3) to head (d40bf8f).
Report is 5 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1816      +/-   ##
============================================
+ Coverage     92.81%   92.83%   +0.01%     
  Complexity      116      116              
============================================
  Files           442      444       +2     
  Lines          4941     4953      +12     
  Branches        234      234              
============================================
+ Hits           4586     4598      +12     
  Misses          308      308              
  Partials         47       47              
Flag Coverage Δ
current 92.56% <100.00%> (+0.01%) ⬆️
current_windows 91.60% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hubtanaka
Copy link
Collaborator Author

@robstoll thanks for your review.
I think I almost done all of tasks and your suggestions.

The one thing I failed is KOTLIN_VERSION=1.9 ./gradlew apiDump.
./gradlew apiDump end with BUILD SUCCESSFUL, but KOTLIN_VERSION=1.9 ./gradlew apiDump failed.

I run ./gradlew.bat apiDump KOTLIN_VERSION=1.9 in console, then got following messages:

Type-safe project accessors is an incubating feature.

Configure project :js-stubs
w: 'kotlin-js' Gradle plugin is deprecated and will be removed in the future.
Please use 'kotlin("multiplatform")' plugin with a 'js()' target instead. See the migration guide: https://kotl.in/t6m3vu

FAILURE: Build failed with an exception.

So I'll find out something wrong tomorrow (just because I'm sleepy a little bit today).

Please let me know if there is something what I should do obviously.

@hubtanaka hubtanaka marked this pull request as ready for review August 20, 2024 16:46
@robstoll
Copy link
Owner

robstoll commented Aug 21, 2024

Can you paste the output in a gist or the like. The excerpt you show is just a warning the build error as such is most likely further above. As a quick fix, you can also copy the changes in:
https://github.com/robstoll/atrium/pull/1816/files#diff-4b296d993af746bdef27f0231160398977048cffed14364841bd8dfda59e0fdfR347

to apis/fluent/atrium-api-fluent/api/using-kotlin-1.9-or-newer/atrium-api-fluent.api
just make sure it is at the exact same place otherwise the apiCheck will fail due to a diff (you can see the current diff here: https://github.com/robstoll/atrium/actions/runs/10475384019/job/29011907775?pr=1816#step:5:616)

and of course, same same for infix and logic

@hubtanaka
Copy link
Collaborator Author

sorry, this is full of the output of my Terminal.

PS C:\Users\tutus\work\github\atrium> ./gradlew.bat apiDump KOTLIN_VERSION=1.9
Type-safe project accessors is an incubating feature.

Configure project :js-stubs
w: 'kotlin-js' Gradle plugin is deprecated and will be removed in the future.
Please use 'kotlin("multiplatform")' plugin with a 'js()' target instead. See the migration guide: https://kotl.in/t6m3vu

FAILURE: Build failed with an exception.

  • What went wrong:
    Task 'KOTLIN_VERSION=1.9' not found in root project 'atrium' and its subprojects.

  • Try:

Run gradlew tasks to get a list of available tasks.
For more on name expansion, please refer to https://docs.gradle.org/8.9/userguide/command_line_interface.html#sec:name_abbreviation in the Gradle documentation.
Run with --stacktrace option to get the stack trace.
Run with --info or --debug option to get more log output.
Run with --scan to get full insights.
Get more help at https://help.gradle.org.

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.9/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD FAILED in 1s
56 actionable tasks: 6 executed, 50 up-to-date
PS C:\Users\tutus\work\github\atrium> ./gradlew.bat KOTLIN_VERSION=1.9 apiDump
Type-safe project accessors is an incubating feature.

Configure project :js-stubs
w: 'kotlin-js' Gradle plugin is deprecated and will be removed in the future.
Please use 'kotlin("multiplatform")' plugin with a 'js()' target instead. See the migration guide: https://kotl.in/t6m3vu

FAILURE: Build failed with an exception.

  • What went wrong:
    Task 'KOTLIN_VERSION=1.9' not found in root project 'atrium' and its subprojects.

  • Try:

Run gradlew tasks to get a list of available tasks.
For more on name expansion, please refer to https://docs.gradle.org/8.9/userguide/command_line_interface.html#sec:name_abbreviation in the Gradle documentation.
Run with --stacktrace option to get the stack trace.
Run with --info or --debug option to get more log output.
Run with --scan to get full insights.
Get more help at https://help.gradle.org.

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.9/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD FAILED in 1s
56 actionable tasks: 6 executed, 50 up-to-date
PS C:\Users\tutus\work\github\atrium>

@hubtanaka
Copy link
Collaborator Author

hubtanaka commented Aug 21, 2024

Thank you. I'll check 2 links in your comment.

also, I feel my Kotlin compiler settings may have something wrong (because I have not care about it from the initial setup.).
2024-08-21_192355

so I'll check this too.

@robstoll
Copy link
Owner

robstoll commented Aug 21, 2024

I see, try the following:

export KOTLIN_VERSION=1.9
./gradlew.bat apiDump

In the end KOTLIN_VERSION needs to be available as environment variable for gradle

@hubtanaka
Copy link
Collaborator Author

hubtanaka commented Aug 21, 2024

@robstoll

export KOTLIN_VERSION=1.9
./gradlew.bat apiDump

I tried it run at least, but ... it seemed not work for me (failed with similar error).
I'm sorry that I'm not even sure whether I could run it correctly or not.

by the way, I think I succeeded it with "Edit Run Configuration: 'atrium [apiDump]" and double click apiDump menu.
2024-08-21_211256
Environment variables was blank when opened. so I filled it with KOTLIN_VERSION=1.9.

anyway, thank you so much for all the help.
I'll re-request review.

@robstoll
Copy link
Owner

I am afraid my Windows knowledge is a bit outdated, I thought it supports an export command as well by now.

Good that it worked via IntelliJ 🙂👍

@robstoll robstoll merged commit a06cdcb into robstoll:main Aug 21, 2024
16 checks passed
@robstoll robstoll linked an issue Aug 21, 2024 that may be closed by this pull request
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature extractor for Iterator.next
2 participants