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.

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

rename ...FeatureExpectationSpec to ...ExpectationsSpec and fuse if existing #329

Closed
robstoll opened this issue Jan 22, 2020 · 11 comments · Fixed by #815
Closed

rename ...FeatureExpectationSpec to ...ExpectationsSpec and fuse if existing #329

robstoll opened this issue Jan 22, 2020 · 11 comments · Fixed by #815
Assignees
@shytnik-igor @tarczynskitomek
Labels
enhancement good first issue Good for newcomers help wanted
Milestone
0.16.0

Comments

@robstoll
Copy link
Owner

robstoll commented Jan 22, 2020 •
edited

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

Code related feature

Wait until #328 is carried out.

I think the distinction between ...ExpectationsSpec and ...FeatureExpectationSpec is more confusing than helpful. Therefore rename ...FeatureExpectationSpec to ...ExpectationsSpec where there isn't an existing ...ExpectationsSpec yet or fuse it into an existing ...ExpectationsSpec
@jGleitz jGleitz mentioned this issue Feb 3, 2020
Merged
@gugatkesheladze
Copy link

i'll work on it

gugatkesheladze pushed a commit to gugatkesheladze/atrium that referenced this issue Jul 12, 2020
robstoll#329 implemented
804f285
@gugatkesheladze gugatkesheladze mentioned this issue Jul 12, 2020
Closed
@robstoll robstoll added the good first issue Good for newcomers label Oct 1, 2020
@tarczynskitomek
Copy link
Contributor

Hi! I'll be happy to help if the issue is still available. Just to reiterate what needs to be done:
Find all .*FeatureAssertionSpec and rename to .*AssertionSpec. If one exists, merge/fuse two specs into one.

@robstoll
Copy link
Owner Author

robstoll commented Oct 4, 2020

Exactly, no need to do everything in one PR. I suggest you rewrite one spec and create a PR

@tarczynskitomek
Copy link
Contributor

Sounds like a plan! I'll start with fusing the example MapFeatureAssertionsSpec, and we'll see how it goes.

There're some minor differences I've noticed in the spec config between MapFeatureAssertionsSpec and MapAssertionsSpec, like different values in nullableMap property, and the latter class using Map<out String, Int> while the former an invariant version Map<String, Int>, so this is some things to be considered while joining both classes.

tarczynskitomek added a commit to tarczynskitomek/atrium that referenced this issue Oct 4, 2020
@tarczynskitomek
Rename MapFeatureAssertionsSpec to MapAssertionsSpec
285f488 
- Join existing Spec files after renaming
- First PR to solve robstoll#329
@tarczynskitomek tarczynskitomek mentioned this issue Oct 4, 2020
Merged
robstoll pushed a commit that referenced this issue Oct 6, 2020
@tarczynskitomek
Rename MapFeatureAssertionsSpec to MapAssertionsSpec (#598)
2ebb3e7 
- Join existing Spec files after renaming
- add `out` to expect(map).contains...values -> add regression to ambiguityTests
- First PR to solve #329
@robstoll
Copy link
Owner Author

@tarczynskitomek unassigning you so that others know they could work on this. Let us know in case you are still working of it and I will re-assign you again.

@tarczynskitomek
Copy link
Contributor

Hi @robstoll sorry for the delay, I'll be able to get back to this at the end of the week - plenty of other tasks right now :-(

@robstoll
Copy link
Owner Author

No worries, I'll let you un-assigned in case someone else want's to contribute in the meantime. I'll re-assign you as soon as you have time again.

@shytnik-igor
Copy link
Collaborator

I didn't find any files ...FeatureAssertionSpec in the project.
Is this issue still actually/correct?

@robstoll
Copy link
Owner Author

The files where renamed from ...AssertionSpec to ...ExpectationSpec

@robstoll robstoll changed the title rename ...FeatureAssertionSpec to ...AssertionSpec and fuse if existing rename ...FeatureExpectationSpec to ...ExpectationsSpec and fuse if existing Feb 16, 2021
@robstoll
Copy link
Owner Author

@shytnik-igor I have updated the description. Let me know in case you don't intend to work on it, then I am un-assigning you again

@shytnik-igor
Copy link
Collaborator

I'll work on this

@shytnik-igor shytnik-igor mentioned this issue Feb 17, 2021
Merged
@robstoll robstoll linked a pull request Feb 18, 2021 that will close this issue
Move PathFeatureExpectationsSpec to PathExpectationsSpec #815
Merged
@robstoll robstoll added this to the 0.16.0 milestone Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shytnik-igor shytnik-igor

@tarczynskitomek tarczynskitomek

Labels
enhancement good first issue Good for newcomers help wanted
Projects
None yet
Milestone
0.16.0
Development

Successfully merging a pull request may close this issue.

Move PathFeatureExpectationsSpec to PathExpectationsSpec shytnik-igor/atrium
4 participants
@robstoll @shytnik-igor @tarczynskitomek @gugatkesheladze