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

Add unit test for the CsvContentExtractionHelper #1396

Closed
nya-elimu opened this issue Aug 15, 2021 · 6 comments · Fixed by #1592
Closed

Add unit test for the CsvContentExtractionHelper #1396

nya-elimu opened this issue Aug 15, 2021 · 6 comments · Fixed by #1592
Assignees
@vrudas
Labels
good first issue Good for newcomers

Comments

@nya-elimu
Copy link
Member

Add unit test for extracting content from the CSV files that happens on application launch. By having unit tests for this, a developer will be notified sooner when introducing breaking changes.
@nya-elimu nya-elimu added the good first issue Good for newcomers label Aug 15, 2021
@vrudas
Copy link
Contributor

vrudas commented Jun 27, 2022

Hi @nya-elimuai as this class contains a lot of responsibilities, is it make sense to split it into separate classes with each own usage? This approach allows for simplifying code complexity and makes it easier for covering tests.

@nya-elimu
Copy link
Member Author

Hi @nya-elimuai as this class contains a lot of responsibilities, is it make sense to split it into separate classes with each own usage? This approach allows for simplifying code complexity and makes it easier for covering tests.

@vrudas Yes, absolutely, I think that's a great idea! Would you be interested in doing this refactoring?

@vrudas
Copy link
Contributor

vrudas commented Jul 2, 2022

@nya-elimuai, have some questions before start:

  • is it suitable to perform such refactoring in this issue?
  • some methods(getWordsFromCsvBackup, getStoryBooksFromCsvBackup) has an extraction logic but result is ignored (by commenting code responsible for value setting into object), such comments should remain? Or can be deleted to make code more readable and easier to support/cover by tests?

@nya-elimu
Copy link
Member Author

nya-elimu commented Jul 3, 2022 •
edited
Loading

@nya-elimuai, have some questions before start:

  • is it suitable to perform such refactoring in this issue?

@vrudas I would recommend this order:

  1. Add unit tests
  2. Refactor the code covered by the unit tests

This way you can be sure that your refactoring does not break anything.

@nya-elimu
Copy link
Member Author

  • some methods(getWordsFromCsvBackup, getStoryBooksFromCsvBackup) has an extraction logic but result is ignored (by commenting code responsible for value setting into object), such comments should remain? Or can be deleted to make code more readable and easier to support/cover by tests?

@vrudas Yes, if you find code that has been commented out, feel free to delete it 🙂

@nya-elimu
Copy link
Member Author

nya-elimu commented Jul 4, 2022 •
edited
Loading

@vrudas You are also welcome to join us in Discord 🙂

@vrudas vrudas self-assigned this Jul 4, 2022
vrudas added a commit to vrudas/webapp that referenced this issue Jul 5, 2022
@vrudas
elimu-ai#1396 Moved extraction of Sound object to a dedicated class
e32dece
vrudas added a commit to vrudas/webapp that referenced this issue Jul 5, 2022
@vrudas
elimu-ai#1396 Created CsvSoundExtractionHelperTest with basic test cases
a52e3fd
vrudas added a commit to vrudas/webapp that referenced this issue Jul 5, 2022
@vrudas
elimu-ai#1396 Covered extraction of valueIpa by tests
6f14fcd
vrudas added a commit to vrudas/webapp that referenced this issue Jul 5, 2022
@vrudas
elimu-ai#1396 Extract logic of preparing CSV content into a method
b854bd8
vrudas added a commit to vrudas/webapp that referenced this issue Jul 5, 2022
@vrudas
elimu-ai#1396 Covered extraction of valueSampa by tests
e4b82ed
vrudas added a commit to vrudas/webapp that referenced this issue Jul 6, 2022
@vrudas
elimu-ai#1396 Covered extraction of diacritic by tests
41a2d6d
vrudas added a commit to vrudas/webapp that referenced this issue Jul 6, 2022
@vrudas
elimu-ai#1396 Covered extraction of soundType by tests
2abd726
vrudas added a commit to vrudas/webapp that referenced this issue Jul 6, 2022
@vrudas
elimu-ai#1396 Add test case for incorrect soundType extraction
df8757f
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Covered extraction of usageCount by tests
fffe995
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Covered catch of IOException by tests
10801fc
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Removed redundant boxing during extraction of diacritic…
fb58f5f 
… value
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Improved logging according SonarQube rule
41ef208 
https://rules.sonarsource.com/java/RSPEC-2629
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Fixed auto-closing of resources
6dd7aa8 
https://rules.sonarsource.com/java/RSPEC-2095
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Hide utility class constructor
fc875c6 
https://rules.sonarsource.com/java/RSPEC-1118
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Add test case of extraction sounds from db/content_TEST…
bfbcd9f 
…/eng/sounds.csv resource
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Create CSVFormat without usage of deprecated API
7ce067a
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Moved extraction of Sound from CsvRecord into a method
a0771ce
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Extract Sounds as immutable collection
3089ee3
vrudas added a commit to vrudas/webapp that referenced this issue Jul 12, 2022
@vrudas
elimu-ai#1396 Moved extraction of Letter object to a dedicated class
d7f4c0a
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Create a test class for CsvLetterExtractionHelperTest
a11cd76
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Add test cases for csv file open-read operations
1210aed
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Covered extraction of Letter text by tests
6a6b5cb
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Covered extraction of Letter diacritic by tests
1036c19
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Covered extraction of Letter usageCount by tests
2a917ab
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Covered extraction of Letter for missing file
bfdcbb2
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Add test case of extraction letters from db/content_TES…
4b19a34 
…T/eng/letters.csv resource
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Improved logging according SonarQube rule
a35e9ed
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Create CSVFormat without usage of deprecated API
21b2ebd
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Fixed auto-closing of resources
10ced5a
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Hide utility class constructor
275cbf7 
https://rules.sonarsource.com/java/RSPEC-1118
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Move Letter object extraction to a dedicated method
5deeec6
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Extract Letters as immutable collection
fc1ee25
vrudas added a commit to vrudas/webapp that referenced this issue Jul 18, 2022
@vrudas
elimu-ai#1396 Removed unused SoundDao argument
9e1bcc9
@vrudas vrudas linked a pull request Jul 18, 2022 that will close this issue
#1396 add unit test for the csv content extraction helper #1592
Merged
@nya-elimu nya-elimu mentioned this issue Jul 19, 2022
Merged
This was referenced Jul 19, 2022
Open
Open
Open
Open
Open
nya-elimu added a commit that referenced this issue Jul 20, 2022
@nya-elimu
Merge pull request #1592 from vrudas/#1396_Add_unit_test_for_the_CsvC…
12cee54 
…ontentExtractionHelper

#1396 add unit test for the csv content extraction helper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vrudas vrudas

Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#1396 add unit test for the csv content extraction helper vrudas/webapp
2 participants
@vrudas @nya-elimu