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

Update parse_job_ad() and add TestParseJobAd() #66

Merged
merged 2 commits into from
May 23, 2023

Conversation

CohenQU
Copy link

@CohenQU CohenQU commented Apr 26, 2023

  1. The parse_job_ad function now prints a warning message instead of exiting if the job ad file cannot be found or read.
  2. A new TestParseJobAd function was added to test the parse_job_ad function in cases where the .job.ad file does not exist.

1. The parse_job_ad function now prints a warning message instead of exiting if the job ad file cannot be found or read.
2. A new TestParseJobAd function was added to test the parse_job_ad function in cases where the .job.ad file does not exist.
@CohenQU CohenQU requested a review from djw8605 April 26, 2023 18:22
@CohenQU CohenQU linked an issue Apr 26, 2023 that may be closed by this pull request
Copy link
Collaborator

@djw8605 djw8605 left a comment

Choose a reason for hiding this comment

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

The commit message isn't descriptive enough. Please update the commit message with the change in the normal code (something about non fatal when reading job ad), and put the change to the test below the top line commit message.

Could you change the jobad test name to something like "TestParseNoJobAd" so it's clear.

…ing with a fatal error when the .job.ad file cannot be found or read. This change makes the function more resilient to unexpected file issues, allowing the program to continue running despite the absence or inaccessibility of the .job.ad file.

Add a new test function named TestParseNoJobAd to specifically test the parse_job_ad function's behavior when the .job.ad file does not exist. This test helps ensure that the function behaves as expected when handling missing or inaccessible job ad files.
@matyasselmeci matyasselmeci requested a review from djw8605 May 19, 2023 16:28
Copy link
Collaborator

@djw8605 djw8605 left a comment

Choose a reason for hiding this comment

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

LGTM

@djw8605 djw8605 merged commit 36c093d into main May 23, 2023
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.

Don't crash if we fail to read the job ad
2 participants