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

Move all regression tests to pytests #21

Open
collado-mike opened this issue Jul 30, 2024 · 2 comments · May be fixed by #545
Open

Move all regression tests to pytests #21

collado-mike opened this issue Jul 30, 2024 · 2 comments · May be fixed by #545
Labels
enhancement New feature or request

Comments

@collado-mike
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Several of the existing regression tests simply do output diff checking. We should migrate them to python tests so we can do better assertions and print more clear error messages when tests fail

Describe the solution you'd like
Move all regression tests to pytests

@collado-mike collado-mike added the enhancement New feature or request label Jul 30, 2024
eric-maynard pushed a commit to eric-maynard/polaris that referenced this issue Jul 30, 2024
eric-maynard pushed a commit to eric-maynard/polaris that referenced this issue Jul 30, 2024
<!-- Please describe your change here and remove this comment -->
This PR covers various improvements to the [recently
merged](snowflakedb/managed-polaris#7) Polaris
CLI. Changes here include:

* Added the `namespaces` command for creating, listing, and dropping
namespaces with the CLI
* Refactored error handling to reduce the proliferation of string
literals
* Added support for the `FILE` storage type
* Added end-to-end regression tests for the CLI
* Various usability and bug fixes
* Support for the `CLIENT_ID` and `CLIENT_SECRET` environment variables
* CLI documentation

## Pre-review checklist
- [ ] I attest that this change meets the bar for low risk without
security requirements as defined in the [Accelerated Risk Assessment
Criteria](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/accelerated-risk-assessment/#eligibility)
and I have taken the [Risk Assessment Training in
Workday](https://wd5.myworkday.com/snowflake/learning/course/6c613806284a1001f111fedf3e4e0000).
- Checking this checkbox is mandatory if using the [Accelerated Risk
Assessment](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/accelerated-risk-assessment/)
to risk assess the changes in this Pull Request.
- If this change does not meet the bar for low risk without security
requirements (as confirmed by the peer reviewers of this pull request)
then a [formal Risk
Assessment](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/risk-assessment/)
must be completed. Please note that a formal Risk Assessment will
require you to spend extra time performing a security review for this
change. Please account for this extra time earlier rather than later to
avoid unnecessary delays in the release process.
- [ ] This change has code coverage for the new code added
eric-maynard pushed a commit to eric-maynard/polaris that referenced this issue Jul 31, 2024
<!-- Please describe your change here and remove this comment -->
This PR covers various improvements to the [recently
merged](https://github.com/snowflakedb/managed-pull/7) Polaris
CLI. Changes here include:

* Added the `namespaces` command for creating, listing, and dropping
namespaces with the CLI
* Refactored error handling to reduce the proliferation of string
literals
* Added support for the `FILE` storage type
* Added end-to-end regression tests for the CLI
* Various usability and bug fixes
* Support for the `CLIENT_ID` and `CLIENT_SECRET` environment variables
* CLI documentation

## Pre-review checklist
- [ ] I attest that this change meets the bar for low risk without
security requirements as defined in the [Accelerated Risk Assessment
Criteria](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/accelerated-risk-assessment/#eligibility)
and I have taken the [Risk Assessment Training in
Workday](https://wd5.myworkday.com/snowflake/learning/course/6c613806284a1001f111fedf3e4e0000).
- Checking this checkbox is mandatory if using the [Accelerated Risk
Assessment](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/accelerated-risk-assessment/)
to risk assess the changes in this Pull Request.
- If this change does not meet the bar for low risk without security
requirements (as confirmed by the peer reviewers of this pull request)
then a [formal Risk
Assessment](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/risk-assessment/)
must be completed. Please note that a formal Risk Assessment will
require you to spend extra time performing a security review for this
change. Please account for this extra time earlier rather than later to
avoid unnecessary delays in the release process.
- [ ] This change has code coverage for the new code added
eric-maynard added a commit to eric-maynard/polaris that referenced this issue Jul 31, 2024
<!-- Please describe your change here and remove this comment -->
This PR covers various improvements to the [recently
merged](https://github.com/snowflakedb/managed-pull/7) Polaris
CLI. Changes here include:

* Added the `namespaces` command for creating, listing, and dropping
namespaces with the CLI
* Refactored error handling to reduce the proliferation of string
literals
* Added support for the `FILE` storage type
* Added end-to-end regression tests for the CLI
* Various usability and bug fixes
* Support for the `CLIENT_ID` and `CLIENT_SECRET` environment variables
* CLI documentation

## Pre-review checklist
- [ ] I attest that this change meets the bar for low risk without
security requirements as defined in the [Accelerated Risk Assessment
Criteria](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/accelerated-risk-assessment/#eligibility)
and I have taken the [Risk Assessment Training in
Workday](https://wd5.myworkday.com/snowflake/learning/course/6c613806284a1001f111fedf3e4e0000).
- Checking this checkbox is mandatory if using the [Accelerated Risk
Assessment](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/accelerated-risk-assessment/)
to risk assess the changes in this Pull Request.
- If this change does not meet the bar for low risk without security
requirements (as confirmed by the peer reviewers of this pull request)
then a [formal Risk
Assessment](https://developer-handbook.m1.us-west-2.aws.app.snowflake.com/docs/reference/security-review/risk-assessment/)
must be completed. Please note that a formal Risk Assessment will
require you to spend extra time performing a security review for this
change. Please account for this extra time earlier rather than later to
avoid unnecessary delays in the release process.
- [ ] This change has code coverage for the new code added
@annafil annafil moved this to Triage in Basic Kanban Board Aug 1, 2024
@MonkeyCanCode
Copy link
Contributor

Thinking to take this one if no one else starts the work yet.

@MonkeyCanCode
Copy link
Contributor

Thinking to take this one if no one else starts the work yet.

@collado-mike here is the PR for this: #545

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants