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

Coverage lines incorrect #28

Closed
yjp20 opened this issue Jan 12, 2021 · 7 comments
Closed

Coverage lines incorrect #28

yjp20 opened this issue Jan 12, 2021 · 7 comments

Comments

@yjp20
Copy link

yjp20 commented Jan 12, 2021

I'm using svelte-jester with preprocess -> babel. When I use collect coverage, the line numbers appear to be wrong. Is this something that is fixable within this library?

@jacob-8
Copy link

jacob-8 commented Mar 18, 2021

I'm using svelte-jester with typescript and my console.log statements are not being printed from the correct lines as mentioned in wallabyjs/public#2669 - this may be a related issue?

I tried adding "compilerOptions": { dev: true }, to the svelte-jester config in my jest.config.js file but that didn't fix anything. See the repro in the linked issue for more.

@smcenlly
Copy link

I investigated this issue raised in our repository (wallabyjs/public#2669) and the issue is lies within svelte-jester. Recent issue #32 is a duplicate of the same problem.

Svelte-jester allows source maps to be generated and used by consumers but does not call the svelte pre-processor (https://github.com/mihar-22/svelte-jester/blob/2337944e0044e460a086aaf6c48e1b2c98d341e7/src/preprocess.js#L8) with sourceMap set to true (see svelte-preprocess options).

To fix this problem, the source maps need to be obtained from the svelte-preprocess and then they need to be merged with the resulting maps from compile (https://github.com/mihar-22/svelte-jester/blob/2337944e0044e460a086aaf6c48e1b2c98d341e7/src/transformer.js#L38) in order for source maps to map back correctly for preprocessed files. I'm not familiar with the svelte/compiler so am unsure as to whether you can pass source maps to it to correctly map back for you or whether you'll need to implement your own merge logic.

Here is a test that reproduces the issue:

const transformer = require("../transformer");
const sourceMap = require("source-map");

describe("typescript line mappings tests", () => {
  it("should correctly map preprocessed files back to original lines", async () => {
    const source =
`<div>Hello {name}</div>

<script lang="typescript">
  export let name: string = ''
  console.log('Test this line');
</script>
`

    const options = { preprocess: true };
    const process = transformer.createTransformer(options).process;
    const result = process(source, 'dummyFile.svelte');
    expect(result.code).toBeDefined();
    expect(result.code).toContain("SvelteComponent");
    expect(result.map).toBeDefined();

    const originalLines = source.replace(/\r/g, '').split('\n');
    const transformedLines = result.code.replace(/\r/g, '').split('\n');

    const consoleLogLineOriginal = originalLines.findIndex(item => item.indexOf('Test this line') !== -1);
    const consoleLogLineTransformed = transformedLines.findIndex(item => item.indexOf('Test this line') !== -1);

    expect(consoleLogLineOriginal).not.toBe(undefined);
    expect(consoleLogLineTransformed).not.toBe(undefined);

    const sourceMapConsumer = await new sourceMap.SourceMapConsumer(result.map);
    let generatedLine = undefined;
    sourceMapConsumer.eachMapping((mapping) => {
      if (mapping.originalLine === consoleLogLineOriginal) {
        if (!generatedLine) {
          generatedLine = mapping.generatedLine;
        } else if (generatedLine !== mapping.generatedLine) {
          throw new Error('Incorrectly maps to multiple lines');
        }
      }
    });

    expect(generatedLine).toBe(consoleLogLineTransformed);
  });
});

@mihar-22
Copy link
Collaborator

Done and released in svelte-jester@1.3.1!

Apologies on the delay I was just busy with work.

@mihar-22
Copy link
Collaborator

Thanks for the detailed information and feedback @smcenlly, much appreciated... helped me debug issue faster.

@juzerzarif
Copy link

Hate to be a party pooper but started using 1.3.1 and I'm now seeing "don't know how to turn this value into a node" from babel for one of my svelte files.

Here's the svelte file

<!-- NoResource.svelte -->
<script lang="ts">
  import Tumbleweed from './Tumbleweed.svelte';
</script>

<div class="h-full w-full flex flex-col justify-center items-center">
  <p class="snapshot-text text-4xl text-center font-semibold opacity-40 mb-6">
    Resource Does Not Exist
  </p>
  <div class="h-2/6 w-full opacity-40">
    <Tumbleweed />
  </div>
</div>

<style>
  .snapshot-text {
    font-family: 'Balsamiq Sans', cursive;
  }
</style>

Error stack trace

 /home/juzerzarif/Documents/terra-wdio-helper/src/webview-ui/common/NoResource.svelte: don't know how to turn this value into a node

      at valueToNode (node_modules/@babel/types/lib/converters/valueToNode.js:90:9)
      at Object.valueToNode (node_modules/@babel/types/lib/converters/valueToNode.js:84:58)
      at Object.exit (node_modules/istanbul-lib-instrument/dist/visitor.js:641:30)
      at PluginPass.exit (node_modules/babel-plugin-istanbul/lib/index.js:158:38)
      at newFn (node_modules/@babel/traverse/lib/visitors.js:175:21)
      at NodePath._call (node_modules/@babel/traverse/lib/path/context.js:55:20)
      at NodePath.call (node_modules/@babel/traverse/lib/path/context.js:42:17)
      at NodePath.visit (node_modules/@babel/traverse/lib/path/context.js:101:8)
      at TraversalContext.visitQueue (node_modules/@babel/traverse/lib/context.js:116:16)
      at TraversalContext.visitSingle (node_modules/@babel/traverse/lib/context.js:85:19)

This is the value that valueToNode receives as argument:

image

Will try to look more into exactly why this file fails and none of the other ones do when I get the time, any help troubleshooting is appreciated!

@mihar-22
Copy link
Collaborator

Ah yucky damn. Open a new issue up please and let's see if it's only related to Babel.

@juzerzarif
Copy link

Opened #33 for this

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

No branches or pull requests

5 participants