microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
66.96k stars 3.67k forks source link

[BUG] Evaluate return partial object #22418

Closed mattveraldi closed 1 year ago

mattveraldi commented 1 year ago

System info

Source code

Config file

import { defineConfig, devices } from '@playwright/test';

/**
 * Read environment variables from file.
 * https://github.com/motdotla/dotenv
 */
// require('dotenv').config();

/**
 * See https://playwright.dev/docs/test-configuration.
 */
export default defineConfig({
  testDir: './tests',
  /* Run tests in files in parallel */
  fullyParallel: true,
  /* 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 ? 2 : 0,
  /* Opt out of parallel tests on CI. */
  workers: process.env.CI ? 1 : undefined,
  /* Reporter to use. See https://playwright.dev/docs/test-reporters */
  reporter: 'html',
  /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
  use: {
    /* Base URL to use in actions like `await page.goto('/')`. */
    // baseURL: 'http://127.0.0.1:3000',

    /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
    trace: 'on-first-retry',
  },

  /* Configure projects for major browsers */
  projects: [
    {
      name: 'chromium',
      use: { ...devices['Desktop Chrome'] },
    },

    // {
    //   name: 'firefox',
    //   use: { ...devices['Desktop Firefox'] },
    // },

    // {
    //   name: 'webkit',
    //   use: { ...devices['Desktop Safari'] },
    // },

    /* Test against mobile viewports. */
    // {
    //   name: 'Mobile Chrome',
    //   use: { ...devices['Pixel 5'] },
    // },
    // {
    //   name: 'Mobile Safari',
    //   use: { ...devices['iPhone 12'] },
    // },

    /* Test against branded browsers. */
    // {
    //   name: 'Microsoft Edge',
    //   use: { ...devices['Desktop Edge'], channel: 'msedge' },
    // },
    // {
    //   name: 'Google Chrome',
    //   use: { ..devices['Desktop Chrome'], channel: 'chrome' },
    // },
  ],

  /* Run your local dev server before starting the tests */
  // webServer: {
  //   command: 'npm run start',
  //   url: 'http://127.0.0.1:3000',
  //   reuseExistingServer: !process.env.CI,
  // },
});

Test file (self-contained)

it('smoke test', async ({ page }) => {
  await page.goto('https://gojs.net/latest/samples/orgChartStatic.html');
  const diagram = await page.locator("#myDiagramDiv");
  let node = await diagram.evaluate((diagramWrapper) => diagramWrapper["goDiagram"].findNodeForKey(1));
  console.warn(node); // it works giving me a partial representation
  console.warn(node.data); // is undefined, but it shouldn't
  let nodeData = await diagram.evaluate((diagramWrapper) => diagramWrapper["goDiagram"].findNodeForKey(1)?.data);
  console.warn(nodeData) // it works
});

Steps

Expected

node.data should work outside of the evaluate context as well as inside it.

Actual

node.data is undefined when accessed outside the evaluate context, but it has other properties.

mxschmitt commented 1 year ago

Investigation notes: Reason for this is because this libray does some prototype hacking internally, which can be tracked down to this minimal reproducible, imagine this below is what we do internally:

const myTest = {}
// without __proto__ it works as expected.
Object.defineProperties(myTest.__proto__, {
    data: {
        get: () => 42,
        set: () => {},
    }
})
// What we do internally to serialise results.
console.log(Object.getOwnPropertyDescriptors(myTest), Object.keys(Object.keys(myTest)))

I recommend for now to access the 'data' property manually directly. Would that work for you?

mattveraldi commented 1 year ago

Investigation notes: Reason for this is because this libray does some prototype hacking internally, which can be tracked down to this minimal reproducible, imagine this below is what we do internally:

const myTest = {}
// without __proto__ it works as expected.
Object.defineProperties(myTest.__proto__, {
    data: {
        get: () => 42,
        set: () => {},
    }
})
// What we do internally to serialise results.
console.log(Object.getOwnPropertyDescriptors(myTest), Object.keys(Object.keys(myTest)))

I recommend for now to access the 'data' property manually directly. Would that work for you?

I see the problem now... What I am doing at the moment is adding "data" manually in the return statement of the evaluate method, but that means that I have to do it for each "getter" in order to have a generic utility function for gojs nodes... Is there any possible workaround to this?

mxschmitt commented 1 year ago

There is unfortunately no easy way around this problem. Keep in mind that the JavaScript inside the page gets serialised with the Node.js runtime, see here: https://playwright.dev/docs/evaluating

So we do a best effort which fails in case of deep prototype getters.

Closing by that for now, since a workaround for this would be to return a property explicitly.

mattveraldi commented 1 year ago

@mxschmitt Ok thank you, just to know if I understood correctly... this the serializer that does the "prototype hacking" you was referring to?

mxschmitt commented 1 year ago

The prototype hacking with the getters is done with this gojs library. I didn't find their source online, but internally they do things like ma.Object.defineProperties(W.prototype, { (minified source). And we internally serialise using this while Object.keys does not work for this library.

mattveraldi commented 1 year ago

Thank you!