-
Notifications
You must be signed in to change notification settings - Fork 711
test: e2e logs downloads test cases #8001
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
Conversation
PR Reviewer Guide 🔍(Review updated until commit 4f7c72a)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 4f7c72a
Previous suggestionsSuggestions up to commit d40292b
|
Resolved conflicts by keeping branch-specific changes: - playwright.yml: kept simplified structure with logsDownloads.spec.js only - .gitignore: kept test-results/.last-run.json entry - logsPage.js: merged both download methods and field management methods 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Persistent review updated to latest commit 4f7c72a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This PR adds comprehensive end-to-end testing for CSV download functionality in the logs interface using Playwright. The changes introduce a new test specification file (logsDownloads.spec.js) that validates various download scenarios including normal downloads, custom date range downloads, and SQL LIMIT query downloads.
The implementation includes significant enhancements to the existing logsPage.js utility file. The ingestion mechanism has been replaced with a direct fetch-based approach using Basic Authentication, providing more control over test data seeding. New filesystem utilities have been added for managing download directories, including automatic cleanup and verification of downloaded CSV files with row count assertions.
The PR also adds a query editor clearing method using keyboard shortcuts to support SQL testing scenarios. The download verification system includes comprehensive checks for file existence, size validation, and content verification with specific row count expectations for different CSV file types (e2e_automate stream expecting 10 rows, custom range downloads expecting 5 rows, and SQL LIMIT queries expecting 2 rows).
Additionally, the GitHub Actions workflow has been updated to include the new test in the Logs-Queries test matrix, and the previously commented-out visualize.spec.js test has been re-enabled in the Dashboards-Charts matrix. The .gitignore file has been reorganized to better handle test artifacts, including the addition of /tests/ui-testing/test-archives for managing generated test files.
Confidence score: 3/5
- This PR introduces filesystem operations and hardcoded credentials that could pose security risks and may cause issues in different testing environments
- Score reflects concerns about hardcoded authentication credentials in test code and potential filesystem permission issues across different CI/CD environments
- Pay close attention to the authentication handling in
logsPage.jsand ensure proper cleanup of downloaded test files
3 files reviewed, 2 comments
…bserve/openobserve into e2e-logsDownloadsTestCases
PR Type
Tests, Enhancement
Description
Add comprehensive logs download E2E tests
Implement download utilities in logs page
Add ingestion via fetch with basic auth
Update CI to run new tests
Diagram Walkthrough
File Walkthrough
logsPage.js
Add ingestion and CSV download utility methodstests/ui-testing/pages/logsPages/logsPage.js
logsDownloads.spec.js
Add comprehensive logs CSV download teststests/ui-testing/playwright-tests/Logs/logsDownloads.spec.js
playwright.yml
Update CI to run new download tests.github/workflows/playwright.yml