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

Integration tests require image-preview to be compiled to run? #81822

Closed
alexdima opened this issue Oct 2, 2019 · 7 comments
Closed

Integration tests require image-preview to be compiled to run? #81822

alexdima opened this issue Oct 2, 2019 · 7 comments
Assignees
Labels
verified Verification succeeded

Comments

@alexdima
Copy link
Member

alexdima commented Oct 2, 2019

We had to comment out this test -- 3f21572

It appears to fail in the integration tests on the remote machine case -- see discussion in release channel. The test is about opening a .png file and there is an error above where the image-preview extension could not be activated.

@alexdima alexdima added this to the September 2019 milestone Oct 2, 2019
@bpasero
Copy link
Member

bpasero commented Oct 2, 2019

via bb0dbb3

@bpasero bpasero closed this as completed Oct 2, 2019
@bpasero
Copy link
Member

bpasero commented Oct 2, 2019

I would argue my fix is just a workaround. There is something I am not understanding:

  • this test did not fail until recently
  • this test does not fail when running against local (only remote)
  • I do not understand why the integration test would try to activate an extension from /Users/vsts/agent/2.155.1/work/1/s/extensions/image-preview/out/extension (as you can see from here)

As such I wonder if something is broken with regards to the packaging of the product for this extension.

Bottom line: the build we run the integration tests against should include all the extensions that are built in. The previous build step is ensuring that.

@mjbvz any clues? Is this a problem with marking the image preview extension UI vs workspace?

@bpasero bpasero reopened this Oct 2, 2019
@bpasero bpasero changed the title Commented out test Integration tests require image-preview to be compiled to run? Oct 2, 2019
@bpasero
Copy link
Member

bpasero commented Oct 2, 2019

I just ran yarn gulp vscode-linux-x64-min locally and could find image-preview is part of the built in extensions. However, the folder structure is that the output is in a dist folder not out. Maybe that is the trouble?

image

@bpasero
Copy link
Member

bpasero commented Oct 2, 2019

I just confirmed that the extension is present on disk before the integration tests start by checking for the content of the image-preview folder. So I am still puzzled about the error.

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 2, 2019

Thanks @bpasero! I cannot explain why this just started failing yesterday.

Based on your analysis, here's an overview of what I think is happening:

  1. We create a production VS Code build. This uses webpack to compile all the extensions.

    The image-preview extension's compiled output goes into dist. As part of this, we should also update the package.json to point to dist:

    data.main = data.main.replace('/out/', /dist/);

  2. In local integration tests, we either:

    • Build all required extensions normally (not using webpack) and then use them for testing. This would work because building without webpack would generate an out directory
    • Run the tests against the production build of vscode that has the correctly patched package.json that points to dist.
  3. In the remote integration tests, we somehow end up running against a production build of the vscode extensions (that have been webpacked so they have a dist directory) but we are using the unpatched package.json that still has "main": "out/..."

/cc @Tyriar I think you worked on the remote integration tests. Do you have any insights on how it is loading our builtin extensions?

@Tyriar
Copy link
Member

Tyriar commented Oct 2, 2019

In the remote integration tests, we somehow end up running against a production build of the vscode extensions

@bpasero sounds like this could be related to running against the prebuilt server via VSCODE_REMOTE_SERVER_PATH? (#80308 being the follow up for smoke tests)

@bpasero bpasero modified the milestones: September 2019, October 2019 Oct 3, 2019
@bpasero
Copy link
Member

bpasero commented Oct 3, 2019

I fully understand now. For remote tests I have to point to a different extensions dir because we need the test resolver extension to be present that is normally not shipped. That is unfortunate, but I have pushed a change to compile ALL extensions before running remote tests.

Only way to resolve this differently is to somehow add the test resolver and then remove it after the tests have run.

//cc @aeschli this means that our remote integration tests currently do not run with the built-in extensions of the product.

@bpasero bpasero closed this as completed Oct 3, 2019
@bpasero bpasero modified the milestones: October 2019, September 2019 Oct 3, 2019
@alexdima alexdima added the verified Verification succeeded label Oct 4, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants