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

feat(build): add mage dagger:run generate:screenshots #1868

Merged
merged 7 commits into from
Jul 17, 2023

Conversation

GeorgeMac
Copy link
Member

@GeorgeMac GeorgeMac commented Jul 14, 2023

Fixes FLI-493

This adds the new mage target: mage dagger:run generate:screenshots.

Currently, for this pass it only defines the screenshots for the Getting Started page.

  • Create Flag
  • Create Variant
  • Create Segment
  • Create Constraint
  • Create Rule

The purpose of this target is to aid in re-generating documentation screenshots as and when we make releases.
The dagger pipeline does the following:

  1. Builds an appropriate container with the playwright dependencies needed
  2. Lists the contents of ui/screenshot/ for .js files.
  3. For each file it runs it with a clean instance of Flipt.
  4. The file optionally will source and import some state from ui/screenshot/fixtures.

Each of these files uses a utility defined in ui/screenshot.js.
This script exports a single function capture which takes a name and a function which operates over a playwright page.
Here is the create_constraint screenshot definition:

const { capture } = require('../screenshot.js');
(async() => {
await capture('create_constraint', async (page) => {
await page.getByRole('link', { name: 'Segments' }).click();
await page.getByRole('link', { name: 'all-users' }).click();
await page.getByRole('button', { name: 'New Constraint' }).click();
await page.getByLabel('Property').fill('admin');
await page
.getByRole('combobox', { name: 'Type' })
.selectOption('BOOLEAN_COMPARISON_TYPE');
await page
.getByRole('combobox', { name: 'Operator' })
.selectOption('notpresent');
});
})();

The name parameter passed to capture is leveraged in two way.

  1. It is used in the resulting name (e.g. create_constraint.png).
  2. It is used to source any import yaml to be seeded before running (e.g. fixtures/create_constraint.yml).

When run from the root of the flipt repository mage dagger:run generate:screenshots will output a directory screenshots in the root of the project.

You can always run flipt yourself and then do the following to experiment with a single screenshot generator:

cd ui
node screenshot/create-constraint.js

@GeorgeMac GeorgeMac requested a review from a team as a code owner July 14, 2023 12:32
@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Merging #1868 (fdf71fa) into main (74dc9c9) will decrease coverage by 0.19%.
The diff coverage is 80.64%.

❗ Current head fdf71fa differs from pull request most recent head 21d70dd. Consider uploading reports for the commit 21d70dd to get more accurate results

@@            Coverage Diff             @@
##             main    #1868      +/-   ##
==========================================
- Coverage   71.64%   71.45%   -0.19%     
==========================================
  Files          58       63       +5     
  Lines        5501     6093     +592     
==========================================
+ Hits         3941     4354     +413     
- Misses       1335     1491     +156     
- Partials      225      248      +23     
Impacted Files Coverage Δ
internal/cmd/auth.go 0.00% <0.00%> (ø)
internal/cmd/grpc.go 0.00% <0.00%> (ø)
internal/cmd/http.go 3.04% <0.00%> (-0.10%) ⬇️
internal/config/authentication.go 71.42% <ø> (ø)
internal/server/audit/audit.go 86.46% <ø> (ø)
internal/server/audit/types.go 77.27% <0.00%> (-22.73%) ⬇️
internal/storage/sql/migrator.go 20.00% <ø> (ø)
internal/server/evaluator.go 45.00% <31.57%> (-49.90%) ⬇️
internal/server/middleware/grpc/middleware.go 63.10% <51.42%> (-6.86%) ⬇️
internal/storage/fs/sync.go 86.02% <65.21%> (-6.84%) ⬇️
... and 10 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

build/testing/ui.go Outdated Show resolved Hide resolved
ui/screenshot.js Outdated

const browser = await chromium.launch({ headless: true });
const context = await browser.newContext({
viewport: { width: 1280, height: 720 },
Copy link
Collaborator

Choose a reason for hiding this comment

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

wdyt about increasing the resolution to something like viewport: { width: 1600, height: 1200 }

Copy link
Collaborator

Choose a reason for hiding this comment

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

or maybe 1440x900? I think thats the resolution I've been using when taking screenshots in Chrome for docs images (have a custom emulated device in the browser setup)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ill give it a go. When I did it locally I got some rather zoomed out shots at higher res. Ill paste here what that gives me.

Copy link
Member Author

Choose a reason for hiding this comment

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

create_constraint
Here is create constraint at the current resolution.

Copy link
Member Author

Choose a reason for hiding this comment

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

create_constraint
Yep you're right. That is nicer. I dunnot what size I had it at before.

@GeorgeMac GeorgeMac requested a review from markphelps July 14, 2023 13:37
Copy link
Collaborator

@markphelps markphelps left a comment

Choose a reason for hiding this comment

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

I think we could rebase this onto main right since there doesn't seem to be anything eval-v2 specific?

@GeorgeMac GeorgeMac force-pushed the gm/dagger-screenshots branch from fdf71fa to acd312b Compare July 17, 2023 12:41
@GeorgeMac GeorgeMac changed the base branch from eval-v2 to main July 17, 2023 12:41
@GeorgeMac GeorgeMac added the automerge Used by Kodiak bot to automerge PRs label Jul 17, 2023
@kodiakhq kodiakhq bot removed the automerge Used by Kodiak bot to automerge PRs label Jul 17, 2023
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Jul 17, 2023

This PR currently has a merge conflict. Please resolve this and then re-add the automerge label.

@GeorgeMac GeorgeMac force-pushed the gm/dagger-screenshots branch from acd312b to b4a324c Compare July 17, 2023 12:49
ui/screenshot.js Show resolved Hide resolved
Copy link
Contributor

@yquansah yquansah left a comment

Choose a reason for hiding this comment

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

Really cool

@GeorgeMac GeorgeMac enabled auto-merge (squash) July 17, 2023 13:09
@GeorgeMac GeorgeMac merged commit 1647217 into main Jul 17, 2023
@GeorgeMac GeorgeMac deleted the gm/dagger-screenshots branch July 17, 2023 13:22
markphelps added a commit that referenced this pull request Jul 17, 2023
* main:
  feat(build): add mage dagger:run generate:screenshots (#1868)
  chore(deps-dev): bump eslint from 8.43.0 to 8.45.0 in /ui (#1878)
  chore(deps-dev): bump postcss from 8.4.24 to 8.4.26 in /ui (#1877)
  chore(deps-dev): bump @playwright/test from 1.35.1 to 1.36.1 in /ui (#1880)
  chore(deps-dev): bump tailwindcss from 3.3.2 to 3.3.3 in /ui (#1876)
  chore(deps-dev): bump eslint-config-airbnb-typescript in /ui (#1879)
  chore(deps): bump github.com/hashicorp/cap from 0.3.1 to 0.3.2 (#1875)
  chore(deps): bump github.com/go-chi/chi/v5 from 5.0.8 to 5.0.10 (#1874)
  chore: remove refs to deprecated io/ioutil (#1867)
  chore(deps-dev): bump @types/react-dom from 18.2.5 to 18.2.7 in /ui (#1864)
  chore(deps-dev): bump jest and @types/jest in /ui (#1851)
  chore(github): move all db unit tests to dagger (#1869)
  chore(deps): bump semver from 6.3.0 to 6.3.1 in /examples/nextjs (#1861)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants