Skip to content
This repository has been archived by the owner on May 2, 2021. It is now read-only.

6 automated tests #7

Merged
merged 7 commits into from
Jan 24, 2017
Merged

6 automated tests #7

merged 7 commits into from
Jan 24, 2017

Conversation

tobiasriedling
Copy link
Contributor

Hi,
the Pull Request contains a JUnit4 Testsuite which boots up AldaServers before running tests and cleans up the servers afterwards.
Also two tests for the AldaClient class. One checks the output of the listProcesses method - called by 'alda list' - and therefore completes the fix for #4. The other tests the checkForExistingServer method. Details in the commit messages.

I was about to write a test for the findExistingSocketForHostAndPort method - also in the fix for #4 - but the codebase could use some refactoring to increase testability.
If JUnit4 is ok i would give it a try and start some refactoring for further tests.

regards tobias

Create a JUnit4 Testsuite for integration tests that sets up a
Testenvironment - starting Alda servers - before running the
integration tests. When the suite finishes it cleans up the
Testenvironment by shutting down the running Alda servers.

Servers getting started and stopped from the TestEnvironment
class by calling the alda exexutable unter test/resources/client/.
If tests rely on the output/behavior of a specific server version
this executable has to be changed/updated accordingly.

see: #6
Test that the ouput of AldaClient listProcesses method - called by
the 'alda list' command - lists all started servers with the correct
port and the workers on the corresponding backend port.

see: #4 'alda list' output buggy when mutliple servers are running
Test that AldaClient checkForExistingServer method detects running
server on host and port.
@daveyarwood
Copy link
Member

Thanks for the PR! This is looking great so far.

I have a few comments:

  • I was able to modify the build.boot in this repo, allowing us to run the JUnit tests by running boot test -- this is convenient since we use Boot across the different Alda Java & Clojure projects.

  • It seems like there is some additional setup to do -- upon running the tests via boot test, I ran into this error:

    Running jUnit tests for class alda.integrationtests.AldaClientTest
    
    Failed:
    
      1) alda.integrationtests.AldaClientTest
         class java.lang.AssertionError: This test class should be started via the IntegrationTestsSuite   which takes care of starting and stopping the TestEnvironment.
           alda.integrationtests.AldaClientTest.checkTestEnvironment  AldaClientTest.java: 26
    
    Finished in 0.006 seconds
    0 tests, 1 failure
    
  • Instead of having to check in and keep track of an alda executable, and having to update it from time to time, it would be great if we could run an Alda server programmatically instead.

    The Alda server classes are included in the classpath by Boot as development dependencies, so I think this may just be a matter of forking a new process and running the main method in alda.Main with args like alda -p 12345 server. The server command is an undocumented way to start an Alda server in the foreground, and this is exactly what happens when you run alda up.

  • Rather than hard-coding in ports to use when testing, it would be better if we found available ports at runtime. I often use this trick to find open ports:

    public static int findOpenPort() throws IOException
    {
        ServerSocket tmpSocket = new ServerSocket(0);
        int portNumber = tmpSocket.getLocalPort();
        tmpSocket.close();
        return portNumber;
    }

    The advantage of doing it this way is that tests will not fail if your system happens to be using port 27716, etc.

AldaClientTest listProccessesOuput fails when run from the terminal
via 'boot test'. The port string parsed from the output stream
contains ansi escapes. Cleaning the string from the ansi escapes
fixes this issue.
@tobiasriedling
Copy link
Contributor Author

Hi,
starting from the end ...

hard-coded port
The issue with the hard-coded port is fixed.

boot test error
The AldaClientTest didn't start the Testenvironment/AldaServer directly, instead the JUnit test suite (IntegrationTestsSuite) started the servers, ran the tests and finally shut down the servers. The AldaClientTest checked 'BeforeClass' that the Testenvironment got started. If not it exited directly with above message. I configured maven here to exclude the integrationtest packages and so it runs only the suite which than runs the Integration Test. I found no quick working solution to configure this in boot-junit.
Tried to move the suite in it's own package and added
junit {:packages '#{alda.testsuites}}
to the task options. But running the boot test task here - with your updated build.boot - failed with the following message: junit: unknown option(s): :packages . So to quick fix this i removed the TestSuite and call the TestEnvironment setup directly from the AldaClientTest 'BeforeClass' and TesEnvironment tearDown 'AfterClass'. This way the boot test task succeeds.

running test server through main method
Because the project is designed as a client server Application the client should perform its integration tests in the long run against different server versions to ensure compatibility. That's why i think it's a better solution to start the released server executables and test against them. To get a quick start i put the executable under test/resources/client - btw. should have named it test/resources/server ... .
All the testenvironment stuff is in theTestenvironment class, there is a variable that points to the executable
private static final String ALDA_EXECUTABLE = "test/resources/client/alda";
The next step could be to check if there is a environment variable like $ALDA_RELEASE_REPO and if it exists use the path from the variable otherwise fallback to the ALDA_EXECUTABLE.
So on the 'build server' this variable could point to some directory like /var/alda/releases/ or a repository server that contains the different alda-s.

@daveyarwood
Copy link
Member

daveyarwood commented Jan 20, 2017

I really like how this is coming along!

I see what you're saying about the release executable. Instead of including the executable in the git repo and hard-coding in the path, what if we had the test suite check for an ALDA_EXECUTABLE environment variable, and fall back to /usr/local/bin/alda if it isn't set? That way, a build server could run tests in succession against different versions in a directory by running something like:

for version in $(ls /var/alda/releases); do
  ALDA_EXECUTABLE=/var/alda/releases/$version boot test
done

This would also require no configuration at all for developers who already have a release of Alda installed to /usr/local/bin, while remaining easy to override if there is a particular release that a developer wants to test against.

@tobiasriedling
Copy link
Contributor Author

Agreed, ALDA_EXECUTABLE -> path to - os dependend - alda installation dir seems reasonable.
I'll give it a try.

My main point here is to keep the setup simple. There are projects out there where you find the bug in minutes but spent hours to set up the environment.
Starting the server from the unit test through the main method gets you to the point where you have to mess around with java.policy files because of permission restrictions. I seem to remember that i read a statement from you about starting a new server process from the boot dev command running in the same direction. But not sure about this.

This project is actual in a state where you simply

  • git pull
  • download dependencies,
  • code fix & test
  • run the test via 'java' command
  • git push

all done with only git a jdk and no configuration.
I think if you keep this door open as long as possible chances are better that other developers stop by and throw you a fix in.

@daveyarwood
Copy link
Member

That's an excellent point -- I agree, it would be good to make sure this project remains approachable for Java developers who may not be used to Boot.

Check for the alda executable in the following order:

1. ALDA_EXECUTABLE environment variable
If this variable is set treat the value as the path to the alda
executable.

2. /usr/local/bin/alda
Check if this installation exists and use the executable.

3. test/resources/server/alda
Fallback to the test/resources/server directory. Include this
directory in the gitignore file. Therefore a alda executable
put in there will not be tracked/submited by git.
@tobiasriedling
Copy link
Contributor Author

Ok - added one more commit.
The Testenvironment class now locates the alda executable via an

  1. ALDA_EXECUTABLE Environment variable. If the variable doesn't exist it checks for an installation under
  2. /usr/local/bin/alda. And if that fails it falls back to
  3. test/resources/server/alda. I removed that executable from the commit and added the directory test/resources/server to the gitignore file. So this file won't get tracked if someone places an alda executable there.

@daveyarwood
Copy link
Member

This looks great! I think we're good to merge. Thanks a lot for all your work on this 🍻

@daveyarwood daveyarwood merged commit 4f331d1 into alda-lang:master Jan 24, 2017
@tobiasriedling tobiasriedling deleted the 6-automated-tests branch January 25, 2017 07:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants