timurstrekalov / saga

Better code coverage tool for JavaScript.
http://timurstrekalov.github.com/saga/
Other
87 stars 25 forks source link

Non-standard jasmine-maven-plugin port results in no tests being found #104

Open WetHippie opened 11 years ago

WetHippie commented 11 years ago

When combined with the jasmine-maven-plugin with a configuration set to a non-default port, the Saga plugin does not correctly find any outputs.

For example, modifying the stock example that's available as a download as such:

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">

4.0.0
    <groupId>com.github.timurstrekalov</groupId>
    <artifactId>saga-with-jasmine-sample</artifactId>
    <version>0.0.1-SNAPSHOT</version>
    <packaging>pom</packaging>

    <name>Using Saga with jasmine-maven-plugin</name>
    <url>http://timurstrekalov.github.com/saga</url>

    <properties>
        <test.server.port>1235</test.server.port>
    </properties>

    <build>
      <plugins>
        <plugin>
            <groupId>com.github.searls</groupId>
            <artifactId>jasmine-maven-plugin</artifactId>
            <version>1.3.1.2</version>
            <executions>
                <execution>
                    <goals>
                        <goal>test</goal>
                    </goals>
                </execution>
            </executions>
            <configuration>
                <serverPort>${test.server.port}</serverPort>
                <keepServerAlive>true</keepServerAlive>
            </configuration>
        </plugin>
        <plugin>
            <groupId>com.github.timurstrekalov</groupId>
            <artifactId>saga-maven-plugin</artifactId>
            <version>1.5.0</version>
            <executions>
                <execution>
                    <goals>
                        <goal>coverage</goal>
                    </goals>
                </execution>
            </executions>
            <configuration>
                <baseDir>http://localhost:${test.server.port}</baseDir>
                <outputDir>${project.build.directory}/coverage</outputDir>
                <noInstrumentPatterns>
                    <pattern>.*/spec/.*</pattern> <!-- Don't instrument specs -->
                </noInstrumentPatterns>
            </configuration>
        </plugin>
      </plugins>
    </build>

Running mvn verify on this results in the following output:

18:42:12.784 [pool-2-thread-1] INFO com.github.timurstrekalov.saga.core.TestRunCoverageStatisticsCallable - Running test at http://localhost:1235 18:42:13.305 [pool-2-thread-1] WARN com.github.timurstrekalov.saga.core.TestRunCoverageStatisticsCallable - No actual test run for file: http://localhost:1235 18:42:13.305 [pool-2-thread-1] INFO com.github.timurstrekalov.saga.core.TestRunCoverageStatisticsCallable - Quitting browser 18:42:13.306 [main] INFO com.github.timurstrekalov.saga.core.DefaultCoverageGenerator - Test run finished

If you take out the port number, the tests correctly find a report to work with.

timurstrekalov commented 11 years ago

First, I think it's always safer for you to use <baseDir>http://localhost:${jasmine.serverPort}</baseDir>, since you have to use the same port that Jasmine is using. Having said that, however, I have to point out that I don't think that's the right way to configure the port for Jasmine, this one works though:

<properties>
    <jasmine.serverPort>8000</jasmine.serverPort>
</properties>

And then use it in baseDir as usual, so that both Jasmine and Saga can pick it up.

WetHippie commented 11 years ago

Note that Jasmine is using the port that I told it to through the element in it's plugin using the exact configuration as in the initial post. If I start it up as jasmine:bdd and then browse that URL in the web browser, everything works as expected. The problem is that the base URL is not being correctly handled in Saga. Saga is seems to be totally ignoring anything other than the hostname in the URL, rather than treating it as a properly configured URL. For example I messed with the URL with a partially qualified root path with the stock port number and Saga also could not find tests in that case either.

There's two possible solutions to the problem here - base URL needs to treat the URL as a proper base and not internally look for a jasmine-maven-plugin specific property. If it does, it gets the right port number specified without having to do any other lookups. The alternative is to change the config property name to be baseFilePath and use it only for files, and then have another property called jasmineHostName and only have the host name (though that will mess with paranoid companies like mine require absolutely everything HTTPS....)

I have started trying to work out what the saga code is doing, but haven't yet got the hang of how it's all put together to make any guesses.

timurstrekalov commented 11 years ago

If you read the jasmine-maven-plugin docs, you'll see that it says the following about the serverPort property:

The jasmine:test goal always uses a random available port so this property is ignored.

So, even though you are completely right in saying that jasmine:bdd uses the property you are setting, it's not going to make any difference to the jasmine:test goal, since it always uses a random port.

This is understandable, since there is no reason for the user to care about the port being used for the jasmine:test goal, since they won't be looking at the tests manually anyway.

Saga, on the other hand, doesn't care about the port/hostname/anything else – it's going to use whatever you provide per the baseDir property as the URL, parsing the whole thing, and definitely not skipping some of the parts :smile_cat:

timurstrekalov commented 11 years ago

BTW, I guess when I said that the aforementioned POM snippet "worked" I was mistaken: somehow, even though Saga reports that it's running tests at port 8000, it runs them correctly, even though the Jetty that the jasmine-maven-plugin starts up spits out completely random ports (which is understandable if you look at the code, since the property would be set after the port is chosen).