Skip to content

Refactor and improve timeline testing infrastructure#86

Merged
taldcroft merged 8 commits intomasterfrom
refactor-modernize
Nov 29, 2024
Merged

Refactor and improve timeline testing infrastructure#86
taldcroft merged 8 commits intomasterfrom
refactor-modernize

Conversation

@taldcroft
Copy link
Copy Markdown
Member

@taldcroft taldcroft commented Nov 12, 2024

Description

This improves the code structure for make_timeline.py and establishes a process for testing.

Highlights:

  • Detailed test process with a couple of new utility scripts to help.
  • Use dynamic file paths instead of hardwired global paths at import time.
    • In "flight" mode (without --test), input data file paths are hardwired to various paths available on HEAD
    • In "test" mode (with --test), input data files must all be in the directory defined by --data-dir (default t_now).
  • Make module importable without side-effects.
  • Add a function to grab some files from the web for testing and put them in the --data-dir directory.
  • Use CxoTime instead of DateTime.
  • Refactor main() into smaller drawing functions. This addresses a longstanding TODO, made easy now with VS code.

Interface impacts

None

Functional testing

On HEAD @ cf7febe

Followed the steps outlined in make_timeline.py for testing on HEAD. This was done with cf7febe. The only changes after that point are documentation or 55cc376 which does not impact the HEAD testing since the --test-get-web option is not used.

After the processing steps, the output timeline states are identical:

$ echo $COMMIT
cf7febe
$ diff t_now/{flight,$COMMIT}/timeline_states.js
$

HTML page outputs:

Local @ 55cc376

Followed the steps outlined in make_timeline.py for local testing on Mac. This was done with 55cc376. The only changes after that point are documentation or adding two test utility scripts.

After the processing steps, the output timeline states are nearly identical:

$ echo $COMMIT
55cc376
$ diff t_now/{flight,$COMMIT}/timeline_states.yaml    
10c10
< p3_avg_now: '81'
---
> p3_avg_now: '80'
3911c3911
<   fluence: 0.02e9
---
>   fluence: 0.01e9

I'm not 100% sure of the reason for the diff but I suspect that the ACIS fluence page updated in the middle of the test. The change in p3_avg_now (which comes from the ACIS fluence page) would propagate to a small change in fluence which is the predicted fluence assuming constant p3_avg_now flux. There are hundreds of states with a dozen state keys each, and they all match exactly except for this one.

The output web pages and timeline.png for the flight and $COMMIT cases are visually identical.

@taldcroft taldcroft requested a review from jeanconn November 15, 2024 11:53
@taldcroft taldcroft changed the title Refactor and modernize timeline code Refactor and improve timeline testing infrastructure Nov 15, 2024
@jeanconn
Copy link
Copy Markdown
Contributor

Regarding the new testing plan - I'd previously written up docs in https://github.com/sot/arc/wiki/Set-up-test-Replan-Central for working in a HEAD dev conda environment to test. I'm wondering if maybe make_timeline.py should just have a link to the arc wiki and have a new section for the doing more atomic testing of make_timeline.py or just a link indicating the other docs to test the whole package do exist.

@taldcroft
Copy link
Copy Markdown
Member Author

@jeanconn - I added a link to the wiki page for full testing of replan central code.

@jeanconn jeanconn requested a review from Copilot November 29, 2024 17:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 4 changed files in this pull request and generated no suggestions.

Files not reviewed (1)
  • pyrightconfig.json: Language not supported

Comment thread make_timeline.py
@taldcroft taldcroft merged commit a96d30e into master Nov 29, 2024
@taldcroft taldcroft deleted the refactor-modernize branch November 29, 2024 19:08
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.

3 participants