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

Dev/weird test #16

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Dev/weird test #16

wants to merge 5 commits into from

Conversation

DawidStuzynski
Copy link

Implement tests for application package

Comment on lines +9 to +19
static jobOffer(args = [:]) {
new JobOffer(
'id' in args.keySet() ? args.id as String : 'test-id',
'title' in args.keySet() ? args.title as String : 'test-title',
'companyName' in args.keySet() ? args.companyName as String : 'test-company',
'experience' in args.keySet() ? args.experience as String : 'test-experience',
'skills' in args.keySet() ? args.skills as List<Skill> : [],
'employmentType' in args.keySet() ? args.employmentType as List<EmploymentType> : [],
'remote' in args.keySet() ? args.remote as boolean : true
)
}
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about simplifying this code?

Suggested change
static jobOffer(args = [:]) {
new JobOffer(
'id' in args.keySet() ? args.id as String : 'test-id',
'title' in args.keySet() ? args.title as String : 'test-title',
'companyName' in args.keySet() ? args.companyName as String : 'test-company',
'experience' in args.keySet() ? args.experience as String : 'test-experience',
'skills' in args.keySet() ? args.skills as List<Skill> : [],
'employmentType' in args.keySet() ? args.employmentType as List<EmploymentType> : [],
'remote' in args.keySet() ? args.remote as boolean : true
)
}
static jobOffer(args = [:]) {
new JobOffer(
args.id as String ?: 'test-id',
args.title as String ?: 'test-title',
args.companyName as String ?: 'test-company',
args.experience as String ?: 'test-experience',
args.skills as List<Skill> ?: [],
args.employmentType as List<EmploymentType> ?: [],
args.remote as boolean ?: true
)
}

Comment on lines +15 to +16
'skills' in args.keySet() ? args.skills as List<Skill> : [],
'employmentType' in args.keySet() ? args.employmentType as List<EmploymentType> : [],
Copy link
Member

Choose a reason for hiding this comment

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

I suggest filling these fields with additional data instead of empty lists.

)
}

static List<JobOffer> createListJobOffers(int count) {
Copy link
Member

Choose a reason for hiding this comment

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

The name of the function "createListJobOffers" may not reflect the intent well, as it may be associated with creating a list and storing it in a database, which is not the case here. I propose the same naming convention as above ie: jobOffersList

Comment on lines +21 to +25
static List<JobOffer> createListJobOffers(int count) {
return (1..count).collect { _ ->
jobOffer()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

You can simplify things a bit

Suggested change
static List<JobOffer> createListJobOffers(int count) {
return (1..count).collect { _ ->
jobOffer()
}
}
static jobOfferList(int count) {
(1..count).collect { jobOffer() }
}


def "publish should chunk job offers and publish them to the queue"() {
given:
List<JobOffer> jobOffers = JobFixture.createListJobOffers(10)
Copy link
Member

@mateuszjanczak mateuszjanczak Jul 8, 2023

Choose a reason for hiding this comment

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

Please use static import

import static it.move2.agent.application.JobFixture.createListJobOffers
Suggested change
List<JobOffer> jobOffers = JobFixture.createListJobOffers(10)
def jobOffers = createListJobOffers(10)

jobBoard1 = Mock()
jobBoard2 = Mock()
queue = Mock()
objectMapper = new ObjectMapper()
Copy link
Member

Choose a reason for hiding this comment

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

How about using the same ObjectMapper that is used in the code from the Bootstrap class? Currently, this ObjectMapper is configured a bit different in tests than in the production version, which may not reveal all the bugs.

def jobs1 = [JobFixture.jobOffer(), JobFixture.jobOffer()]

jobBoard1.getJobs() >> jobs1
jobBoard2.getJobs() >> { throw new RuntimeException("Test error message") }
Copy link
Member

Choose a reason for hiding this comment

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

How is error handling carried out in the application? I haven't seen anything throwing an exception.

jobBoard2.getJobs() >> { throw new RuntimeException("Test error message") }

when:
updater.execute()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
updater.execute()
updater.execute()


then:
thrown(RuntimeException)
0 * queue.pub(_) >> {}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
0 * queue.pub(_) >> {}
0 * queue.pub(_)

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.

2 participants