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

[prober] delete rid-related tests from prober #745

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

Shastick
Copy link
Contributor

@Shastick Shastick commented Aug 7, 2024

Remove the RID parts of the prober, which are now entirely covered by the qualifier.

Closes #744

@barroco
Copy link
Contributor

barroco commented Aug 7, 2024

@Shastick could you please check references to the prober in the dss repo documentation and make sure we properly update it in order to maintain the same level of testing by documenting the uss qualifier usage?
Example: https://github.com/interuss/dss/blob/master/build/pooling.md#establishing-a-pool-with-first-instance

@Shastick
Copy link
Contributor Author

Shastick commented Aug 8, 2024

@Shastick could you please check references to the prober in the dss repo documentation and make sure we properly update it in order to maintain the same level of testing by documenting the uss qualifier usage? Example: https://github.com/interuss/dss/blob/master/build/pooling.md#establishing-a-pool-with-first-instance

I did a first pass on the DSS repo (see interuss/dss#1066): I essentially mentioned the qualifier wherever the prober was being mentioned.

@mickmis
Copy link
Contributor

mickmis commented Aug 23, 2024

Could you tell me how you ensured that all of those prober tests are now covered by USS qualifier scenarios? I do see that all of those tests seem to have a matching scenario, but then did you make a pass to ensure that all test cases are covered? Thanks!

@Shastick
Copy link
Contributor Author

Shastick commented Aug 23, 2024

Could you tell me how you ensured that all of those prober tests are now covered by USS qualifier scenarios? I do see that all of those tests seem to have a matching scenario, but then did you make a pass to ensure that all test cases are covered? Thanks!

See #744 for the details.

The relevant part:

The following gap analysis is based on both the older InterUSS Backlog as well as a manual check of the prober files and their corresponding qualifier scenarios for RID.

Copy link
Contributor

@mickmis mickmis left a comment

Choose a reason for hiding this comment

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

Change LGTM, but shouldn't we wait for the dss repo to use the uss_qualifier for probing instead of the prober before merging?
Because in the meantime if we need to change something in the prober for the CI in the DSS, we will have an issue.

@Shastick
Copy link
Contributor Author

Yes, I'd wait before merging this. We also still need a release of the monitoring repo in order to be able to use the qualifier.

@Shastick
Copy link
Contributor Author

interuss/dss#1045 has landed. @mickmis we should have proper coverage on the DSS repo without the prober as of this moment and this PR can be merged.

@Shastick Shastick requested a review from mickmis August 27, 2024 12:41
Copy link
Contributor

@mickmis mickmis left a comment

Choose a reason for hiding this comment

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

LGTM
Branch needs to be updated though. @Shastick can I just update the branch through github or will you take care of it? Actually not really needed, my bad.

@mickmis mickmis merged commit d492674 into interuss:main Aug 27, 2024
20 checks passed
@Shastick Shastick deleted the rid-prober-retirement branch August 27, 2024 15:21
github-actions bot added a commit to brandoncorrea/monitoring that referenced this pull request Aug 31, 2024
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.

[rid-prober] migrate prober logic entirely to uss_qualifier scenarios, then remove prober
3 participants