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.

Sign up for GitHub

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

Jump to bottom

[uss_qualifier] Migrate test_isa_validation prober to the new qualifier #238

Merged
merged 1 commit into from
Oct 13, 2023
Merged

[uss_qualifier] Migrate test_isa_validation prober to the new qualifier #238

merged 1 commit into from
Oct 13, 2023

Conversation

Shastick
Copy link
Contributor

@Shastick Shastick commented Oct 10, 2023 •
edited
Loading

Migrates the previous test_isa_validation prober test to the new qualifier.

This attempts to support both v19 and v22a.

@Shastick Shastick force-pushed the dss0030-migrate-prober branch 2 times, most recently from 59e7d00 to 077b65f Compare October 12, 2023 09:56
@Shastick Shastick changed the title [uss_qualifier] migrate some parts of the test_isa_validation prober to the new qualifier [uss_qualifier] migrate test_isa_validation prober to the new qualifier Oct 12, 2023
@Shastick Shastick force-pushed the dss0030-migrate-prober branch from 077b65f to 5920de0 Compare October 12, 2023 10:04
@Shastick Shastick marked this pull request as ready for review October 12, 2023 10:18
@Shastick
Copy link
Contributor Author

Open question: should we also port the assertions about error messages from the prober, eg like here?

@Shastick Shastick force-pushed the dss0030-migrate-prober branch 4 times, most recently from fbf09b9 to bf3ca1a Compare October 12, 2023 13:32
@Shastick Shastick marked this pull request as draft October 12, 2023 13:33
@Shastick Shastick force-pushed the dss0030-migrate-prober branch from bf3ca1a to 19e835d Compare October 12, 2023 13:39
@Shastick Shastick marked this pull request as ready for review October 12, 2023 13:39
@Shastick Shastick force-pushed the dss0030-migrate-prober branch 2 times, most recently from f65eefe to 75ef70d Compare October 13, 2023 07:55
Shastick
Shastick commented Oct 13, 2023
View reviewed changes
monitoring/uss_qualifier/scenarios/astm/netrid/common/dss/isa_validation.py Outdated
Comment on lines 26 to 31
HUGE_VERTICES: List[s2sphere.LatLng] = [
s2sphere.LatLng.from_degrees(lng=130, lat=-23),
s2sphere.LatLng.from_degrees(lng=130, lat=-24),
s2sphere.LatLng.from_degrees(lng=132, lat=-24),
s2sphere.LatLng.from_degrees(lng=132, lat=-23),
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a new resource for passing such arbitrary list of vertices. Otherwise this PR is ready for review.

@Shastick Shastick force-pushed the dss0030-migrate-prober branch 3 times, most recently from d6102a3 to 3424e38 Compare October 13, 2023 08:53
@Shastick
Copy link
Contributor Author

Open question: should we also port the assertions about error messages from the prober, eg like here?

Summary of our chat on the issue: content in error messages should only be enforced if the standard requires it. This does not seem to be the case here, so we'll simply look at http codes.

@Shastick Shastick force-pushed the dss0030-migrate-prober branch from 3424e38 to ea91590 Compare October 13, 2023 09:02
@Shastick Shastick force-pushed the dss0030-migrate-prober branch from ea91590 to 0572ed3 Compare October 13, 2023 09:13
BenjaminPelletier
BenjaminPelletier approved these changes Oct 13, 2023
View reviewed changes
monitoring/uss_qualifier/scenarios/astm/netrid/v19/dss/isa_validation.md

#### ISA huge area check

Attempting to put a too large ISA should result in a 400.
Copy link
Member

Choose a reason for hiding this comment

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

We don't have to (and shouldn't) do everything in a single PR so no need to change anything in this PR, but eventually we'll want to mention the requirement(s) each check violates. Pretty much everything in this scenario will be DSS0030,a after #245 (simply DSS0030 currently)

@BenjaminPelletier BenjaminPelletier changed the title [uss_qualifier] migrate test_isa_validation prober to the new qualifier [uss_qualifier] Migrate test_isa_validation prober to the new qualifier Oct 13, 2023
@BenjaminPelletier BenjaminPelletier merged commit 08502ce into interuss:main Oct 13, 2023
9 checks passed
@Shastick Shastick mentioned this pull request Nov 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminPelletier BenjaminPelletier BenjaminPelletier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Shastick @BenjaminPelletier