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 automated test tasks to run on EST timezone #1588

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

basiliskus
Copy link
Contributor

@basiliskus basiliskus commented Nov 15, 2024

Description

The Automated Tests are currently scheduled to run around midnight UTC, but our team working hours are between Pacific Time and Eastern Time. Changing to use EST to be closer to actual off hours

Issue

#1582

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1582 - Partially compliant

Fully compliant requirements:

  • Pick an appropriate time (e.g. 12am PT)
  • Update the scheduled tasks in the github workflows
    • .github/workflows/automated-staging-test-run.yml
    • .github/workflows/automated-staging-test-submit.yml
  • Update the time zone in rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java

Not compliant requirements:

  • Make sure the blob fetching, organization and cleanup for Automated Tests run as expected
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Time Zone Update
The time zone has been updated to 'America/Los_Angeles' but it's unclear if this change impacts the functionality of the blob fetching, organization, and cleanup processes as required by the ticket.

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Enhancement
Make the timezone setting configurable to enhance flexibility and maintainability

Ensure that the hardcoded timezone "America/Los_Angeles" is appropriate for all use
cases or consider making the timezone configurable to accommodate different
environments or future changes.

rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java [20]

-private static final ZoneId TIME_ZONE = ZoneId.of("America/Los_Angeles");
+private static final ZoneId TIME_ZONE = ZoneId.of(System.getenv("DEFAULT_TIMEZONE") != null ? System.getenv("DEFAULT_TIMEZONE") : "America/Los_Angeles");
Suggestion importance[1-10]: 7

Why: Making the timezone configurable is a good practice for maintainability and flexibility, especially if the application may need to support multiple regions in the future.

7
Possible issue
Confirm the accuracy of the cron schedule to ensure tests run at the intended times

Verify that the new cron schedule "0 10 * * 2-6" correctly corresponds to the
intended 2am PST, considering the UTC offset and daylight saving changes.

.github/workflows/automated-staging-test-run.yml [5]

-- cron: "0 10 * * 2-6"  # Tuesday to Saturday at 2am PST (10am UTC) - two hours after `automated-staging-test-submit` runs
+- cron: "0 10 * * 2-6"  # Adjusted to correct UTC offset for 2am PST
Suggestion importance[1-10]: 6

Why: Verifying the cron schedule's accuracy is crucial to ensure that integration tests run as intended, though the suggestion does not provide a specific improvement but rather a verification step.

6
Ensure the cron schedule reflects the correct time for Midnight PST, including daylight saving considerations

Confirm that the new cron schedule "0 8 * * 2-6" for the staging test submission is
correctly set for Midnight PST, especially considering daylight saving time
adjustments.

.github/workflows/automated-staging-test-submit.yml [5]

-- cron: "0 8 * * 2-6"  # Tuesday to Saturday at Midnight PST (8am UTC)
+- cron: "0 8 * * 2-6"  # Verified and adjusted for correct timing at Midnight PST
Suggestion importance[1-10]: 6

Why: Ensuring the accuracy of the cron schedule for the staging test submission is important for correct operation, though like the previous suggestion, it mainly prompts a verification without suggesting a specific improvement.

6

@basiliskus basiliskus changed the title Update automated test tasks to run on PST timezone Update automated test tasks to run on CST timezone Nov 15, 2024
@basiliskus basiliskus marked this pull request as ready for review November 15, 2024 23:09
Copy link
Member

@halprin halprin left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but I defer to Jeff and his concerns.

Copy link

sonarcloud bot commented Nov 18, 2024

@basiliskus basiliskus changed the title Update automated test tasks to run on CST timezone Update automated test tasks to run on EST timezone Nov 18, 2024
@basiliskus basiliskus merged commit 5ae6a27 into main Nov 19, 2024
17 checks passed
@basiliskus basiliskus deleted the story/1582/reschedule-tasks branch November 19, 2024 15:16
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.

5 participants