Skip to content

test: add basic integration test for CRUD #339

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
Merged
merged 3 commits into from
Apr 2, 2025
Merged

test: add basic integration test for CRUD #339

merged 3 commits into from
Apr 2, 2025

Conversation

@sgoral-splunk
Copy link
Contributor

@sgoral-splunk sgoral-splunk commented Mar 24, 2025 •
edited
Loading

Issue number: ADDON-78485

PR Type

What kind of change does this PR introduce?

  • Feature
  • Bug Fix
  • Refactoring (no functional or API changes)
  • Documentation Update
  • Maintenance (dependency updates, CI, etc.)

Summary

Changes

Add test for basic CRUD operation for RestHandler.

User experience

No impact.

Checklist

If an item doesn't apply to your changes, leave it unchecked.

Review

  • self-review - I have performed a self-review of this change according to the development guidelines
  • Changes are documented. The documentation is understandable, examples work (more info)
  • PR title and description follows the contributing principles
  • meeting - I have scheduled a meeting or recorded a demo to explain these changes (if there is a video, put a link below and in the ticket)

Tests

See the testing doc.

  • Unit - tests have been added/modified to cover the changes
  • Integration - tests have been added/modified to cover the changes
  • Smoke - tests have been added/modified to cover the changes
  • UI - tests have been added/modified to cover the changes
  • coverage - I have checked the code coverage of my changes (see more)

artemrys
artemrys requested changes Mar 27, 2025
View reviewed changes
tests/integration/test_rest_handler_handler.py Outdated
Comment on lines 29 to 30
admin = "admin"
admin_password = "Chang3d!"
Copy link
Member

@artemrys artemrys Mar 27, 2025

Choose a reason for hiding this comment

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

This is a temporary change, right? :)

Copy link
Contributor

@kkedziak-splunk kkedziak-splunk Apr 2, 2025

Choose a reason for hiding this comment

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

Better to revert it or at least getenv with a default value: os.getenv("SPLUNK_ADMIN", "admin")

Copy link
Contributor Author

@sgoral-splunk sgoral-splunk Apr 2, 2025

Choose a reason for hiding this comment

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

Yes, by mistake I pushed my local config :)

kkedziak-splunk
kkedziak-splunk reviewed Apr 2, 2025
View reviewed changes
@sgoral-splunk sgoral-splunk marked this pull request as ready for review April 2, 2025 12:40
@sgoral-splunk sgoral-splunk requested a review from a team as a code owner April 2, 2025 12:40
@artemrys artemrys merged commit b8b5996 into main Apr 2, 2025
20 checks passed
@artemrys artemrys deleted the test/basic_crud_tests branch April 2, 2025 12:52
@github-actions github-actions bot locked and limited conversation to collaborators Apr 2, 2025
@srv-rr-github-token
Copy link
Contributor

srv-rr-github-token commented Sep 17, 2025

🎉 This PR is included in version 8.0.1-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@artemrys artemrys artemrys approved these changes

@kkedziak-splunk kkedziak-splunk kkedziak-splunk approved these changes

Assignees

No one assigned

Labels

released on @beta

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sgoral-splunk @srv-rr-github-token @artemrys @kkedziak-splunk