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

[347] Refactor tests to remove hardcoded rupture sets #357

Merged
merged 3 commits into from
Dec 6, 2024

Conversation

voj
Copy link
Collaborator

@voj voj commented Dec 4, 2024

As part of #347

This PR removes most uses of hard-coded alpine-vernon rupture sets or solutions.

These changes have brought a bug in our hazard calculations to light: #355 Once this issue is fixed, the last usage of an alpine-vernon solution can be removed.

There is a small number of other hard-coded files remaining in tests, and these can be removed in a separate PR. This one is already big enough.

General changes in this PR:

  • Generate rupture sets and solutions on the fly for tests.
  • Make inversion runners and hazard calculator accept ArchiveInput, rupture sets, and solutions instead of only Files and Strings. This is to avoid having to write temporary files during testing.

@voj voj self-assigned this Dec 4, 2024
@voj voj changed the title Chore/347 update tests [347] Refactor tests to remove hardcoded rupture sets Dec 4, 2024
@voj voj force-pushed the chore/347-update-tests branch from f76388a to 90445b4 Compare December 4, 2024 20:31
@voj voj marked this pull request as ready for review December 4, 2024 20:44
@voj voj requested review from chrisbc and chrisdicaprio December 4, 2024 20:44
Copy link
Contributor

@chrisdicaprio chrisdicaprio left a comment

Choose a reason for hiding this comment

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

looks good; much tidier!

@@ -215,7 +215,6 @@ public static class GridHazards {
public static void main(String[] args) throws DocumentException, IOException {

NZSHM22_HazardCalculatorBuilder builder = new NZSHM22_HazardCalculatorBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't you need to set a solution file anymore? or maybe a better question is why did you need to set one before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We still need a solution file, this is now broken. I'll uncomment the next line. Good catch!

@voj voj merged commit 75d7612 into main Dec 6, 2024
1 check passed
@voj voj deleted the chore/347-update-tests branch December 6, 2024 02:49
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