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

Fixed that URL does not contain the "empty" #103

Merged
merged 2 commits into from
Feb 19, 2015
Merged

Fixed that URL does not contain the "empty" #103

merged 2 commits into from
Feb 19, 2015

Conversation

magnolia-k
Copy link

When we generate a coverage report of source code that is placed directly below 'src/main/scala/' , the link to the report of HTML, 'empty' string is inserted that, it is not even properly reaction click.

Example:

source -> src/main/scala/Version.scala
URL in index.html -> file:///Users/xxxx/dev/xxxx/target/scala-2.11/scoverage-report/(empty)/Version.scala.html
Generated HTML -> /Users/xxxx/dev/scala/xxxx/target/scala-2.11/scoverage-report/Version.scala.html

Order to remove the hierarchy of unnecessary 'empty' from a URL.

Because the generated HTML links does not work correctly.
@gslowikowski
Copy link
Member

This reverts #9. I wonder what problem was fixed by that PR.

@Kwestor
Copy link
Contributor

Kwestor commented Feb 14, 2015

Could you please include a unit tests for your fix? It's very helpful for tracking regressions.

@magnolia-k
Copy link
Author

Thank you comment @gslowikowski @Kwestor

Since there is no test of 'ScoverageHtmlWriter.scala', and try to write, including the other part. So, I want you to wait a little.

PR #9 is not an absolute path the HTML URL of, is intended to change the relative path, this PR does not change for that point. I just only to such does not include the 'empty' in the URL.

We acquires the coverage module following such directory structure...

src/main/scala/Class1.scala
src/main/scala/subdir/Class2.scala

Following such HTML is output.

scoverage-report/overview.html
scoverage-report/Class1.scala.html
scoverage-report/subdir/Class2.scala.html

At this time, since the URL of overview.html has become as follows, I just want to fix this.

<a href="(empty)/Class1.scala.html"> -> invalid!
<a href="nest/Class2.scala.html">

Since overview.html and Class1.scala.html is created in the same directory, if delete the extra 'empty', it becomes as to function as the correct link.

@magnolia-k
Copy link
Author

Added test code.

sksamuel added a commit that referenced this pull request Feb 19, 2015
Fixed that URL does not contain the "empty"
@sksamuel sksamuel merged commit 8674ab1 into scoverage:master Feb 19, 2015
@gslowikowski
Copy link
Member

I haven't time to test it before. When running tests on Windows ScoverageHtmlWriterTest fails with stacktrace:

[info] ScoverageHtmlWriterTest:
[info] - HTML coverage report has been created correctly *** FAILED ***
[info]   java.io.FileNotFoundException: D:\home\gs\Temp\452f0fc7-b29c-477b-8b21-53274b41316b\G:\scm.gslowikowski\github.git\gslowikowski\scalac-scoverage-plugin\branches\using-release-plugin\scalac-scoverage-plugin\target\scala-2.11\test-classes\scoverage\forHtmlWriter\src\main\scala\Class1.scala.html (Nazwa pliku, nazwa katalogu lub skĹ'adnia etykiety woluminu jest niepoprawna)
[info]   at java.io.FileOutputStream.open(Native Method)
[info]   at java.io.FileOutputStream.<init>(FileOutputStream.java:221)
[info]   at java.io.FileOutputStream.<init>(FileOutputStream.java:171)
[info]   at java.io.FileWriter.<init>(FileWriter.java:90)
[info]   at scoverage.IOUtils$.writeToFile(IOUtils.scala:37)
[info]   at scoverage.report.ScoverageHtmlWriter.scoverage$report$ScoverageHtmlWriter$$writeFile(ScoverageHtmlWriter.scala:42)
[info]   at scoverage.report.ScoverageHtmlWriter$$anonfun$scoverage$report$ScoverageHtmlWriter$$writePackage$1.apply(ScoverageHtmlWriter.scala:35)
[info]   at scoverage.report.ScoverageHtmlWriter$$anonfun$scoverage$report$ScoverageHtmlWriter$$writePackage$1.apply(ScoverageHtmlWriter.scala:35)
[info]   at scala.collection.immutable.List.foreach(List.scala:381)
[info]   at scoverage.report.ScoverageHtmlWriter.scoverage$report$ScoverageHtmlWriter$$writePackage(ScoverageHtmlWriter.scala:35)
[info]   ...

@Kwestor
Copy link
Contributor

Kwestor commented Feb 24, 2015

See #79 (comment), it's probably the same problem.

It only manifests on Windows, as on Linux paths are wrong, but parsable.

@gslowikowski
Copy link
Member

I analyzed this case and I see many problems with this PR:

  1. I've created regular project in src/test/resources/scoverage/forHtmlWriter directory, generaged scoverage report with v.1.0.4 and found that there are no (empty) directories (this is because of 2.) so this is not a good test for this issue
  2. In this test source files are not aligned with package names. This is currently required by scoverage (should be written somewhere of just fixed). If this requirement is not met, some links are not generated properly (I don't remember which ones).
  3. ScoverageHtmlWriterTest test not written properly. It's perhaps based on CoberturaXmlWriterTest and that test doesn't look good to me too. What's wrong - the paths. In coverage files they are always in absolute, canonical form. For this test to be written properly the code block:
    val class2 = getClass.getResource("forHtmlWriter/src/main/scala/subdir/Class2.scala").getFile()
    val class1 = getClass.getResource("forHtmlWriter/src/main/scala/Class1.scala").getFile()

    coverage.add(
      Statement(class2,
        Location("coverage.sample", "Class2","Class", ClassType.Object, "msg_test", class2),
        2, 57, 77, 4, "scala.this.Predef.println(&quot;test code&quot;)",
        "scala.Predef.println", "Apply", false, 0)
    )

    coverage.add(
      Statement(class1,
        Location("coverage.sample", "Class1","Class", ClassType.Object, "msg_coverage", class1),
        1, 61, 96, 4, "scala.this.Predef.println(&quot;measure coverage of code&quot;)",
        "scala.Predef.println", "Apply", false, 0)
    )

should be changed to:

    val class2 = new File(getClass.getResource("forHtmlWriter/src/main/scala/subdir/Class2.scala").getFile()).getCanonicalPath()
    val class1 = new File(getClass.getResource("forHtmlWriter/src/main/scala/Class1.scala").getFile()).getCanonicalPath()

    coverage.add(
      Statement(class2,
        Location("coverage.sample", "Class2", "Class2", ClassType.Class, "msg_test", class2),
        2, 60, 80, 4, "scala.this.Predef.println(\"test code\")",
        "scala.Predef.println", "Apply", false, 0)
    )

    coverage.add(
      Statement(class1,
        Location("coverage.sample", "Class1", "Class1", ClassType.Class, "msg_coverage", class1),
        1, 64, 99, 4, "scala.this.Predef.println(\"measure coverage of code\")",
        "scala.Predef.println", "Apply", false, 0)
    )

So, ScoverageHtmlWriterTest test should be fixed. Anyway, I still don't know If reverting #9 is good decision, and what problem was fixed by that change.

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.

4 participants