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

fix safari scroll #1663

Merged
merged 18 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .github/workflows/linting-and-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -373,16 +373,14 @@ jobs:
uses: actions/cache@v3
with:
path: "~/.cache/ms-playwright"
key: ${{ runner.os }}-playwright-${{ env.PLAYWRIGHT_VERSION }}
key: ${{ runner.os }}-playwright-${{ env.PLAYWRIGHT_VERSION }}-chromium-firefox-webkit

- name: Install Playwright binaries/dependencies
if: steps.playwright-cache.outputs.cache-hit != 'true'
# if more browsers are added, will need to modify the "npx playwright install" command
# https://stackoverflow.com/questions/65900299/install-single-dependency-from-package-json-with-yarn
run: |
yarn add "@playwright/test@${{ env.PLAYWRIGHT_VERSION }}"
npx playwright install --with-deps chromium firefox
npx playwright install-deps
npx playwright install --with-deps chromium firefox webkit
joeyorlando marked this conversation as resolved.
Show resolved Hide resolved

- name: Await k8s pods and other resources up
uses: jupyterhub/action-k8s-await-workloads@v1
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Fix team search when filtering resources by @vadimkerr ([#1680](https://github.com/grafana/oncall/pull/1680))
- Fix issue when trying to scroll in Safari ([#415](https://github.com/grafana/oncall/issues/415))

## v1.2.6 (2023-03-30)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import { createIntegrationAndSendDemoAlert } from '../utils/integrations';
import { createOnCallSchedule } from '../utils/schedule';

test('we can create an oncall schedule + receive an alert', async ({ page }) => {
// this test does a lot of stuff, lets give it adequate time to do its thing
test.slow();

const escalationChainName = generateRandomValue();
const integrationName = generateRandomValue();
const onCallScheduleName = generateRandomValue();
Expand Down
2 changes: 1 addition & 1 deletion grafana-plugin/integration-tests/utils/forms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const clickButton = async ({
dataTestId,
}: ClickButtonArgs): Promise<void> => {
const baseLocator = dataTestId ? `button[data-testid="${dataTestId}"]` : 'button';
const button = (startingLocator || page).locator(`${baseLocator} >> text=${buttonText}`);
const button = (startingLocator || page).locator(`${baseLocator}:not([disabled]) >> text=${buttonText}`);
Copy link
Contributor

@joeyorlando joeyorlando Mar 30, 2023

Choose a reason for hiding this comment

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

this should make some of the flaky schedules tests more reliable.. I think the issue was that the tests were trying to click on the "Add rotation" button, which according to this screenshot, starts off disabled. So it would click the disabled button and then expect for something to become available.. but obviously it wouldn't because the button click did nothing.

By adding this, Playwright should wait until the desired button is enabled before trying to click it:
Screenshot 2023-03-30 at 16 12 25

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, create rotation button is disabled when creation form is already opened and it's always disabled for non-web schedules, i'm not sure what happened in that case...


await button.waitFor({ state: 'visible' });
await button.click();
Expand Down
20 changes: 7 additions & 13 deletions grafana-plugin/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const config: PlaywrightTestConfig = {
testDir: './integration-tests',
globalSetup: './integration-tests/globalSetup.ts',
/* Maximum time one test can run for. */
// TODO: set this back to 60 when GSelect component is refactored
timeout: 90 * 1000,
expect: {
/**
Expand All @@ -28,10 +27,7 @@ const config: PlaywrightTestConfig = {
/* Fail the build on CI if you accidentally left test.only in the source code. */
forbidOnly: !!process.env.CI,
/* Retry on CI only */
retries: process.env.CI ? 1 : 0,
// TODO: when GSelect component is refactored, run using 3 workers
// locally use one worker, on CI use 3
// workers: process.env.CI ? 3 : 1,
retries: process.env.CI ? 3 : 0,
workers: 1,
/* Reporter to use. See https://playwright.dev/docs/test-reporters */
reporter: 'html',
Expand Down Expand Up @@ -64,14 +60,12 @@ const config: PlaywrightTestConfig = {
...devices['Desktop Firefox'],
},
},

// TODO: enable tests on Safari once the scroll bug when creating an integration is patched
// {
// name: 'webkit',
// use: {
// ...devices['Desktop Safari'],
// },
// },
{
name: 'webkit',
use: {
...devices['Desktop Safari'],
},
},

/* Test against mobile viewports. */
// {
Expand Down
3 changes: 2 additions & 1 deletion grafana-plugin/src/img/grafanaGlobalStyles.css
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* Navigation/Layout */

.drawer-content {
.drawer-content,
.rc-drawer-content {
overflow: auto !important; /* fix https://github.com/grafana/oncall/issues/415 */
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,7 @@ class EscalationChainsPage extends React.Component<EscalationChainsPageProps, Es
return (
<>
<Block withBackground className={cx('header')}>
<Text
size="large"
editable
onTextChange={this.handleEscalationChainNameChange}
data-testid="escalation-chain-name"
>
<Text size="large" onTextChange={this.handleEscalationChainNameChange} data-testid="escalation-chain-name">
{escalationChain.name}
</Text>
<div className={cx('buttons')}>
Expand Down