-
Notifications
You must be signed in to change notification settings - Fork 5
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
Enhance Testing Framework #488 #489 #490 #491 #492 #493
base: staging
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
.env.local.template
Outdated
@@ -1,4 +0,0 @@ | |||
# Next secrets |
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.
why you deleted this
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.
this has been deleted by mistake
|
||
setup("authentication", async ({ page }) => { | ||
setup("admin authentication", async () => { |
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.
why it is different tests?
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.
Now the BrowserWrapper functionally creates the page instead of using Playwright's built-in methods.
}) | ||
const roles = [ | ||
{ name: 'admin' }, | ||
// { name: 'readwrite' }, |
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.
missing roles?
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.
@@ -18,7 +18,7 @@ test.describe('Settings Tests', () => { | |||
}) | |||
|
|||
Data.inputDataRejectsZero.forEach(({ input, description, expected }) => { | |||
test(`Modify ${roles.maxQueuedQueries} via API validation via UI: Input value: ${input} description: ${description}`, async () => { | |||
test(`@admin Modify ${roles.maxQueuedQueries} via API validation via UI: Input value: ${input} description: ${description}`, async () => { |
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.
is there a test that check that the other users cannot get to this page?
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.
No, we don't have any tests that check this, we should have a test which check if other users can access settings page, and we should have other tests which checks if other users can use the api to make any changes in the settings
@@ -16,7 +16,7 @@ test.describe('Settings Tests', () => { | |||
await browser.closeBrowser(); | |||
}) | |||
|
|||
test("Add one new user -> validating user exists in the users list", async () => { | |||
test("@admin Add one new user -> validating user exists in the users list", async () => { |
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.
is there a test that check that the other users cannot get to this page?
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.
No, we don't have any tests that check this, we should have a test which check if other users can access settings page, and we should have other tests which checks if other users can use the api to make any changes in the settings
e2e/tests/signOut.spec.ts
Outdated
@@ -3,22 +3,30 @@ import urls from '../config/urls.json' | |||
import BrowserWrapper from "../infra/ui/browserWrapper"; | |||
import navBarComponent from '../logic/POM/navBarComponent' | |||
|
|||
test.describe('NavBar Tests', () => { | |||
let browser : BrowserWrapper; | |||
const roles = [ |
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.
can't this be declared for all tests?
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.
yes, i will change it.
No description provided.