-
Notifications
You must be signed in to change notification settings - Fork 1
Securesign 922 #53
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
Securesign 922 #53
Conversation
Reviewer's GuideThis PR replaces the rod-based UI tests with a Playwright-Go framework, introduces a BrowserTest abstraction for reusable search routines, adds multi-browser support via environment variables, simplifies test data extraction using regex, and updates dependencies and build scripts accordingly. Class diagram for BrowserTest abstraction and Playwright integrationclassDiagram
class BrowserTest {
+browserName: string
+page: playwright.Page
+Launch()
+Navigate(url string)
+SearchByEmail(email string)
+SearchByHash(hash string)
+SearchByLogIndex(index string)
+SearchByUUID(uuid string)
+SearchByCommitSHA(sha string)
+TakeScreenshot(name string)
}
class Playwright {
+Install()
+LaunchBrowser(name string)
+Close()
}
BrowserTest --|> Playwright : uses
Class diagram for updated environment variable configurationclassDiagram
class Values {
+SetDefault(key string, value string)
+AutomaticEnv()
}
Values : +TestFirefox = "TEST_FIREFOX"
Values : +TestSafari = "TEST_SAFARI"
Values : +TestEdge = "TEST_EDGE"
Values : +HeadlessUI = "HEADLESS_UI"
Values : +RegistryImage = "REGISTRY_IMAGE"
Values : +TsaURL = "TSA_URL"
Values : +ManualImageSetup = "MANUAL_IMAGE_SETUP"
Values : +CosignImage = "COSIGN_IMAGE"
Values : +DockerRegistryUsername = "REGISTRY_USERNAME"
Values : +DockerRegistryPassword = "REGISTRY_PASSWORD"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Waiting for upstream to resolve email search limitation issue |
027a977 to
55fedbd
Compare
cec9819 to
2f35090
Compare
Signed-off-by: Kristián Da Costa Menezes <113685889+kdacosta0@users.noreply.github.com>
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.
Hey @kdacosta0 - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `scripts/Makefile:40` </location>
<code_context>
-tuf-repo:
- @echo "Running manual TUF repository test..."
- go test -v ./test/tuftool
+setup:
+ @echo "Installing Playwright dependencies..."
+ @npm install @playwright/test
+ @npx playwright install --with-deps
+ @go run github.com/playwright-community/playwright-go/cmd/playwright install
-.PHONY: build test test-tuf-repo all
</code_context>
<issue_to_address>
The setup target mixes npm and Go-based Playwright installation.
Standardize on either npm or Go for Playwright installation to prevent redundancy and version conflicts.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
setup:
@echo "Installing Playwright dependencies..."
@npm install @playwright/test
@npx playwright install --with-deps
@go run github.com/playwright-community/playwright-go/cmd/playwright install
=======
setup:
@echo "Installing Playwright dependencies..."
@npm install @playwright/test
@npx playwright install --with-deps
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `pkg/api/values.go:28` </location>
<code_context>
RegistryImage = "REGISTRY_IMAGE"
TsaURL = "TSA_URL"
HeadlessUI = "HEADLESS_UI"
+ TestFirefox = "TEST_FIREFOX"
+ TestSafari = "TEST_SAFARI"
+ TestEdge = "TEST_EDGE"
// 'DockerRegistry*' - Login credentials for 'registry.redhat.io'.
</code_context>
<issue_to_address>
New browser test environment variables default to 'true'.
Defaulting these variables to 'true' could cause tests to run and fail if the browsers aren't present. Consider using 'false' as the default or clearly documenting the browser requirements.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
Values.SetDefault(TestFirefox, "true")
Values.SetDefault(TestSafari, "true")
Values.SetDefault(TestEdge, "true")
=======
Values.SetDefault(TestFirefox, "false")
Values.SetDefault(TestSafari, "false")
Values.SetDefault(TestEdge, "false")
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Kristián Da Costa Menezes <113685889+kdacosta0@users.noreply.github.com>
Signed-off-by: Kristian Da Costa Menezes <kdacosta@redhat.com>
Summary by Sourcery
Migrate and refactor the Rekor Search UI end-to-end tests to use Playwright for real cross-browser support, update configuration and build scripts, and clean up obsolete dependencies
Enhancements:
Build: