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

Using AssertJ assertions on entire exercises #2147

Closed
thethoughtcode opened this issue Jul 23, 2022 · 16 comments
Closed

Using AssertJ assertions on entire exercises #2147

thethoughtcode opened this issue Jul 23, 2022 · 16 comments
Labels
good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed policy x:action/improve Improve existing functionality/content x:knowledge/none No existing Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:size/small Small amount of work x:type/content Work on content (e.g. exercises, concepts)

Comments

@thethoughtcode
Copy link

thethoughtcode commented Jul 23, 2022

The test classes use mix of Junit asserts and AssertJ. Since AssertJ assertions are more robust and extensive as compared to Junit5.

Like for example, usage of assertThatExceptionOfType from AssertJ is much more readable as compare to assertThrows from JUnit 5.

assertThatExceptionOfType(RuntimeException.class)
         .isThrownBy(() -> { throw new RuntimeException(new IllegalArgumentException("boom!")); })
         .havingCause()
         .withMessage("boom!");

We can migrate all the asserts from Junit to AsserJ.

Working on this issue

This issue affects several exercises. If you want to contribute to this issue, you don't have to change this in all exercises, feel free to submit small PRs fixing this issue for a single exercise or a small subset of exercises!

@andrerfcsantos
Copy link
Member

This sounds like a good idea, thanks for suggesting!

I edited the description with how to contribute to this issue.

@andrerfcsantos andrerfcsantos added good first issue Good for newcomers x:action/improve Improve existing functionality/content x:knowledge/none No existing Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:type/content Work on content (e.g. exercises, concepts) x:size/small Small amount of work labels Jul 30, 2022
@anandhakrishnanaji
Copy link

I'm working on this

@suraj-mandal
Copy link

Hi, this is my first time, I will be contributing towards open source. May I work on this issue, and migrate some of the tests from junit5 to AssertJ?

@danielmrcl
Copy link
Contributor

Convert exception assertions to AssertJ in classes starting with 'A' and 'B'. #2156

@jennylia
Copy link
Contributor

I am working on the Haming

@jennylia
Copy link
Contributor

#2178 is my PR

@jennylia
Copy link
Contributor

#2181 another PR

@smcg468
Copy link
Contributor

smcg468 commented Nov 16, 2022

Hi, converting markdown from assertEquals to assertThat

@sanderploegsma
Copy link
Contributor

I am in favour of this change, but it does mean that we have to revise the existing policy in POLICIES.md that states that we prefer JUnit assertions over AssertJ.

@pavanbaloju
Copy link
Contributor

Hello @sanderploegsma, I would like to contribute to a few exercises.

@pavanbaloju
Copy link
Contributor

Raised one more PR for few more tests. Please review !

@pavanbaloju
Copy link
Contributor

Raised one more PR for few more tests. Please review!

@pavanbaloju
Copy link
Contributor

Raised one more PR #2513 which covers all the pending tests which needs to be updated

@sanderploegsma
Copy link
Contributor

I found three more practice exercises to update.

  • Grade School
  • Tree Building
  • Zipper

@keszocze I know you're working on Zipper, care to pick up the other two as well?

@keszocze
Copy link
Contributor

keszocze commented Oct 6, 2023

edit: Yes of course I can also change those.

But there are some more (and I will also change them):

grep -l 'org.junit.Assert' practice/*/src/test/java/*.java                                                                                                                                                   practice/diffie-hellman/src/test/java/DiffieHellmanTest.java
practice/grade-school/src/test/java/SchoolTest.java
practice/octal/src/test/java/OctalTest.java
practice/strain/src/test/java/StrainTest.java
practice/tree-building/src/test/java/BuildTreeTest.java
practice/trinary/src/test/java/TrinaryTest.java
practice/zipper/src/test/java/ZipperTest.java

and I sure can also touch them. But that might take until Monday.

Btw, in my last PR I was told that the Diffie Hellman exercise was deprecated and that this tells me how to spot that. But looking at the config.json shows none of the mentioned things in that file. I can't even find a status entry in any of the json files: grep status */*/.(meta|approaches)/*.json yields no results. How can I spot the deprecated exercises?

@sanderploegsma
Copy link
Contributor

sanderploegsma commented Oct 6, 2023

I'm guessing you're looking at each exercise's .meta/config.json file, but the exercise statuses are kept in config.json at the root of this repository. Here's a list of the practice exercises currently deprecated:

cat config.json | jq -r '.exercises.practice[] | select(.status == "deprecated") | .slug' | sort
accumulate
beer-song
binary
diffie-hellman
hexadecimal
octal
strain
trinary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed policy x:action/improve Improve existing functionality/content x:knowledge/none No existing Exercism knowledge required x:module/practice-exercise Work on Practice Exercises x:size/small Small amount of work x:type/content Work on content (e.g. exercises, concepts)
Projects
None yet
Development

No branches or pull requests

11 participants