Skip to content

Comments

Add Java analyzer for Annalyns Infiltration#130

Merged
sanderploegsma merged 4 commits intoexercism:mainfrom
Gyoo:main
Feb 16, 2024
Merged

Add Java analyzer for Annalyns Infiltration#130
sanderploegsma merged 4 commits intoexercism:mainfrom
Gyoo:main

Conversation

@Gyoo
Copy link
Contributor

@Gyoo Gyoo commented Feb 6, 2024

Fixes #98

I also added the analyzer comments to the website-copy repository : exercism/website-copy#2323

@Gyoo Gyoo requested a review from a team as a code owner February 6, 2024 15:06
@Gyoo Gyoo changed the title Add Java analyzer for Annalyns Infiltration (#98) Add Java analyzer for Annalyns Infiltration Feb 6, 2024
@manumafe98
Copy link
Contributor

Hey Gyoo, thanks for submitting a PR! I would suggest to change this to draft, and wait until this major reformat of the analyzer is applied

@Gyoo
Copy link
Contributor Author

Gyoo commented Feb 8, 2024

Thank you for your feedback. I will monitor the other issue for now :)

@Gyoo Gyoo marked this pull request as draft February 8, 2024 13:07
@sanderploegsma
Copy link
Contributor

Hi @Gyoo, we have finished refactoring the test setup, so feel free to update your branch with the changes on main and finishing this PR 👍

@Gyoo Gyoo marked this pull request as ready for review February 15, 2024 09:05
Copy link
Contributor

@sanderploegsma sanderploegsma left a comment

Choose a reason for hiding this comment

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

Please also add a couple of smoke tests for this exercise to the tests/ directory. We aim to have at least two: one with an exemplar solution and one or more with a solution that is designed to receive some feedback from the analyzer.

Have a look at the smoke tests for the other exercises to see what is expected.

@Gyoo
Copy link
Contributor Author

Gyoo commented Feb 15, 2024

@sanderploegsma I made the changes you requested but I'm wondering if I'm missing something regarding smoke tests : I have one exemplar solution and 4 other tests to trigger the different analyzer cases. Do I need something more ?

@sanderploegsma
Copy link
Contributor

The main difference between the JUnit tests in src/test/java and the smoke tests in tests/ is that the former is used to verify the correctness of the analyzer and achieve test coverage, but it does not run the full analyzer end-to-end like it would run on Exercism. This is why we have the smoke tests: they work by running the analyzer on a solution as a Docker container, similar to how the website runs it.

@Gyoo
Copy link
Contributor Author

Gyoo commented Feb 15, 2024

Thank you for your feedback. I'll have a second look at this then!

Copy link
Contributor

@sanderploegsma sanderploegsma left a comment

Choose a reason for hiding this comment

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

Other than my one open question this looks good to go, thanks!

Note for maintainers: make sure the website-copy PR is merged first

@sanderploegsma sanderploegsma merged commit 898a1f2 into exercism:main Feb 16, 2024
@sanderploegsma
Copy link
Contributor

Thanks for your contribution @Gyoo! 🎉

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.

annalyns-infiltration: implement analyzer

4 participants