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

chore: convert screenshots.js to screenshots.ts #30758

Merged
merged 16 commits into from
Dec 20, 2024
Merged
Changes from 2 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
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
const _ = require('lodash')
const mime = require('mime')
const path = require('path')
const Promise = require('bluebird')
const dataUriToBuffer = require('data-uri-to-buffer')
const Jimp = require('jimp')
const sizeOf = require('image-size')
const colorString = require('color-string')
const sanitize = require('sanitize-filename')
let debug = require('debug')('cypress:server:screenshot')
const plugins = require('./plugins')
const { fs } = require('./util/fs')

import _ from 'lodash'
import Debug from 'debug'
import mime from 'mime'
import path from 'path'
import Promise from 'bluebird'
import dataUriToBuffer from 'data-uri-to-buffer'
import Jimp from 'jimp'
import sizeOf from 'image-size'
import colorString from 'color-string'
import sanitize from 'sanitize-filename'
import * as plugins from './plugins'
import { fs } from './util/fs'

let debug = Debug('cypress:server:screenshot')
const RUNNABLE_SEPARATOR = ' -- '
const pathSeparatorRe = /[\\\/]/g

// internal id incrementor
let __ID__ = null
let __ID__: string | null = null

// many filesystems limit filename length to 255 bytes/characters, so truncate the filename to
// the smallest common denominator of safe filenames, which is 255 bytes. when ENAMETOOLONG
Expand All @@ -32,20 +33,35 @@ const MIN_PREFIX_BYTES = 64

// when debugging logs automatically prefix the
// screenshot id to the debug logs for easier association
// @ts-ignore
debug = _.wrap(debug, (fn, str, ...args) => {
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
return fn(`(${__ID__}) ${str}`, ...args)
})

jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
const isBlack = (rgba) => {
interface RGBA {
r: number
g: number
b: number
a: number
}

const isBlack = (rgba: RGBA): boolean => {
return `${rgba.r}${rgba.g}${rgba.b}` === '000'
}

const isWhite = (rgba) => {
const isWhite = (rgba: RGBA): boolean => {
return `${rgba.r}${rgba.g}${rgba.b}` === '255255255'
}

const intToRGBA = function (int) {
const obj = Jimp.intToRGBA(int)
interface RGBAWithName extends RGBA {
name?: string
isNotWhite?: boolean
isWhite?: boolean
isBlack?: boolean
}

const intToRGBA = function (int: number): RGBAWithName {
const obj: RGBAWithName = Jimp.intToRGBA(int) as RGBAWithName

if (debug.enabled) {
obj.name = colorString.to.keyword([
Expand Down Expand Up @@ -115,7 +131,7 @@ const captureAndCheck = function (data, automate, conditionFn) {

return (attempt = function () {
tries++
const totalDuration = new Date() - start
const totalDuration = new Date().getTime() - start.getTime()

debug('capture and check %o', { tries, totalDuration })

Expand Down Expand Up @@ -187,7 +203,7 @@ const pixelConditionFn = function (data, image) {
return passes
}

let multipartImages = []
let multipartImages: { data, image, takenAt, __ID__ }[] = []

const clearMultipartState = function () {
debug('clearing %d cached multipart images', multipartImages.length)
Expand Down Expand Up @@ -246,7 +262,7 @@ const stitchScreenshots = function (pixelRatio) {

debug(`stitch ${multipartImages.length} images together`)

const takenAts = []
const takenAts: string[] = []
let heightMarker = 0
const fullImage = new Jimp(fullWidth, fullHeight)

Expand Down Expand Up @@ -278,6 +294,7 @@ const getBuffer = function (details) {

return Promise
.promisify(details.image.getBuffer)
// @ts-ignore
.call(details.image, Jimp.AUTO)
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -316,6 +333,7 @@ const ensureSafePath = function (withoutExt, extension, overwrite, num = 0) {
}

// path does not exist, attempt to create it to check for an ENAMETOOLONG error
// @ts-ignore
return fs.outputFileAsync(fullPath, '')
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
.then(() => fullPath)
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
.catch((err) => {
Expand Down Expand Up @@ -350,6 +368,7 @@ const getPath = function (data, ext, screenshotsFolder, overwrite) {
.chain(data.titles)
.map(sanitizeToString)
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
.join(RUNNABLE_SEPARATOR)
// @ts-ignore
.concat([])
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what the next line is even doing; _.concat only operates on arrays, and "someString".concat([]) === "someString". The next line can likely be removed, but I'd make sure the values are what they're expected to be if you do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ryan had some thoughts on what this is doing here: #30758 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

@cacieprins @ryanthemanuel Yah....it does not like any changes to this area. I'm going to leave it alone. 😬 I don't really understand why it's needed either.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I think I get what it's doing, but lodash types don't like it even though you can do it.

Copy link

Choose a reason for hiding this comment

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

Hey, while I was working on this part of code in my PR, I found that this is done so that type of the variable is same in both if and else branch. I guess that is why it breaks, as the type for name variable becomes different in case of if and else branches, and the remaining code that uses names breaks on one type or other. I fixed it by replacing the concat by putting it in array.

Relevant lines from my PR https://github.com/cypress-io/cypress/pull/30673/files#diff-471ce7b0d88e3cdc126b6c5e781d074d86a828e5a7d88b9584db3385be09e569R88-R97

.value()
}
Expand All @@ -376,7 +395,7 @@ const getPathToScreenshot = function (data, details, screenshotsFolder) {
return getPath(data, ext, screenshotsFolder, data.overwrite)
}

module.exports = {
export = {
crop,

getPath,
Expand Down Expand Up @@ -468,8 +487,10 @@ module.exports = {

return getBuffer(details)
.then((buffer) => {
// @ts-ignore
return fs.outputFileAsync(pathToScreenshot, buffer)
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
}).then(() => {
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
// @ts-ignore
return fs.statAsync(pathToScreenshot).get('size')
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
}).then((size) => {
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
const dimensions = getDimensions(details)
Expand All @@ -492,7 +513,7 @@ module.exports = {
},

afterScreenshot (data, details) {
const duration = new Date() - new Date(data.startTime)
const duration = new Date().getTime() - new Date(data.startTime).getTime()

details = _.extend({}, data, details, { duration })
details = _.pick(details, 'testAttemptIndex', 'size', 'takenAt', 'dimensions', 'multipart', 'pixelRatio', 'name', 'specName', 'testFailure', 'path', 'scaled', 'blackout', 'duration')
Expand Down
Loading