CW Testing platform: Tests are broken if stdout is not terminated

[Voileexperiments]

Try this:

using System;
using NUnit.Framework;

[TestFixture]
public class KataTestClass
{
    [Test]
    public void Test()
    {
        Console.Write("foobar");
        Assert.AreEqual(true, false);
    }
}

However, if the line Console.Write("foobar"); is replaced by either:
Console.Write("foobar\n");
or
Console.WriteLine("foobar");

The test no longer breaks. This suggests that unterminated stdout breaks the testing suite.

[Voileexperiments]

I just found that JS exhibits the same bug as well, so it's clearly a problem at the CW side rather than language specific:

Test.expect(true);
process.stdout.write('foobar');
Test.expect(false);
process.stdout.write('foobar\n');
Test.expect(false);
process.stdout.write('foobar');
Test.expect(false);

[kazk]

Duplicate of #867.

We should open a meta-issue on CLI repo listing languages having this issue using checkbox. I was going to open a PR for each language, but completely forgot about this while fixing other stuff.

Java and Go should be fixed now.

• Java (Java now forces passed/failed message onto new lines codewars-runner-cli#336)
• Go ([Go] Prepend LF to output commands codewars-runner-cli#339)
• C (codewars/codewars-runner-cli@25e5609)
• C#
• JavaScript

[Voileexperiments]

Java is not fixed yet:

import org.junit.Test;
import static org.junit.Assert.assertEquals;
import org.junit.runners.JUnit4;

public class SolutionTest {
    @Test
    public void testSomething() {
        assert(true);
        System.out.print("foo");
        assert(false);
        System.out.print("bar");
        assert(true);
    }
}

[kazk]

Thanks @Voileexperiments.
I don't know much about jvm-runner and that PR doesn't include tests to confirm that it fixes the issue... and Travis had errors so I can't see the outputs.

@jhoffner is the fix deployed?

[Voileexperiments]

I tested the same thing against some more languages I know a bit of:

C, Ruby: test won't fail but it still throws an error and ends prematurely anyway

//c, prematurely ends with signal code 11 (segfault)
#include <stdio.h>
#include <criterion/criterion.h>
Test(foo, bar) {
  cr_assert_eq(true, true);
  fputs("foo",stdout);
  cr_assert_eq(true, false);
  fputs("bar",stdout);
  cr_assert_eq(true, true);
}

#ruby
#`expect': Value is not what was expected (Test::Error)
#	from `<main>'
Test.expect(true)
print 'foo'
Test.expect(false)
print 'bar'
Test.expect(true)

C++: not fixed

//c++
Describe(foo){
    It(bar){
        Assert::That(true);
        std::cout << "foo";
        Assert::That(false);
        std::cout << "bar";
        Assert::That(true);
    }
};

Python: not fixed (both in Python 2 and 3)

#Python 2
import sys

test.expect(True);
sys.stdout.write('foo')
test.expect(False);
sys.stdout.write('bar')
test.expect(True);

#Python 3
test.expect(True);
print('foo', end='')
test.expect(False);
print('bar', end='')
test.expect(True);

[kazk]

I commented out the System.out.print and it still passes with assert(false).

import org.junit.Test;
import static org.junit.Assert.assertEquals;
import org.junit.runners.JUnit4;

public class SolutionTest {
    @Test
    public void testSomething() {
        // assert(true);
        // System.out.print("foo");
        assert(false);
        // System.out.print("bar");
        // assert(true);
    }
}

[Voileexperiments]

Oh yeah, that seems to be the wrong thing to call. I always wanted to use the equivalent of Test.expect whenever it exists on a language, but it seems that not all languages have it.

Seems like Java is good to go?

import org.junit.Test;
import static org.junit.Assert.assertEquals;
import org.junit.runners.JUnit4;

public class SolutionTest {
    @Test
    public void testSomething() {
        assertEquals(true,true);
        System.out.print("foo");
        assertEquals(true,true);
        System.out.print("bar");
        assertEquals(true,false);
        System.out.print("foobar");
        assertEquals(true,true);
    }
}

[Voileexperiments]

Oh, by the way, I don't think "unterminated stdout causes tests to end prematurely from the middle" would be the right behaviour, so while those languages above don't allow cheesing the tests, it's probably still an issue.

[kazk]

@Voileexperiments

C, Ruby: test won't fail but it still throws an error and ends prematurely anyway

Is this what you get for C?

This seems to be handling the line feed correctly. Some test frameworks stops at first failure. If this is what you mean, I think we should open a separate issue to make this easier to track. Also, I'm going to open a meta-issue for LF on CLI.

[Voileexperiments]

Hmm, yeah I guess that's what I get for not learning the test frameworks for other languages properly. :P I thought that was caused by the test terminated prematurely due to errors caused by unterminated stdout (just like some of those cough implicit dependency issues in JS)

So, it seems that only Python is exhibiting the problem. But then, I'm not familiar with all those other languages, so they might have the same problem as well? I think you know more languages than me :P, so maybe you can check them out?

[kazk]

@Voileexperiments Thanks for all the info.
I'm going to close this because I made a meta-issue codewars/codewars-runner-cli#410 to keep track the issues caused by line-feeds. I'll try to look into other languages as well.
Also, expected behaviors for each language and test framework needs to be documented.

[jhoffner]

@jhoffner is the fix deployed?

I haven't released any changes in the last 2 weeks. I'm debating doing a release tomorrow since I'll be around this weekend to monitor, but I'm slightly anxious to release anything right before I leave on vacation. It looks like everything we have queued is pretty minor so far so the risk should be low.

[kazk]

It looks like everything we have queued is pretty minor so far so the risk should be low.

Yeah, last 2 weeks were mostly fixing minor issues.
codewars-runner-cli/compare/master@{2weeks}...master

The risk seems low but it's still a change so I understand. Those changes might not be worth having some anxiety during your vacation 😎

[Voileexperiments]

@jhoffner please do release those changes! #928 has returned to plague many people again, and reports are flying everywhere. It'd be the worst if we have to sit through this for a whole month. :P

[kazk]

@Voileexperiments what do you mean by those changes? I thought we're talking about the changes on codewars-runner-cli repo for the past few weeks and most are ESLint fixes.

[Voileexperiments]

oh, I thought the fix for #928 is along the changes and not pushed to the main site yet.

[kazk]

[deleted]

webpackJsonp is defined in manifest.js but www. seems to lack that file.

[kazk]

Maybe the urls of manifest.js and app.js on www. is somehow outdated?
vendor.js has same 18ea8d283566a3354553 and loads on both.

www.

• manifest.3168bb31eb870c38e096.js (404)
• vendor.18ea8d283566a3354553.js (200)
• app.70b963111d561c75ba17.js (404)

preview.

• manifest.afa0a6c3b75342436815.js (200)
• vendor.18ea8d283566a3354553.js (200)
• app.b0a4a22d98fad4abf413.js (200)

[kazk]

@jhoffner if you're using -p flag for production build, webpack/webpack#368 might help.

[jhoffner]

Thanks @kazk. The files are being built correctly, however the original CDN issues (when going through CloudFlare) were super odd in that it would just stop loading assets after a while. Seems to be related to some sort of expiration.

The same issue happened here, but in this case I am now deploying using the Firebase CDN, where I specifically place the files on the CDN (instead of CloudFlare's transparent caching). However this time it makes more sense because it turns out Firebase CDN deployments completely replace any CDN assets that were there before (even though they still charge you for the space, which is lame).

For now my fix is going to have to be to continue to keep track of older assets and make sure they get deployed along with any new assets (which is easy since we are using hashes). In this case, I deployed to staging and it wiped out production's assets, and once the cached files expired on their servers the assets were no longer available to be retrieved again.

I would still love to know why the hell this issue was happening in the first place (when we were using CloudFlare). It never happened before when we were using Gulp/Riot, so I think you are right that it's probably something with the webpack config. I'm just not sure what or how though - since it seemingly is an issue of the files magically disappearing from the server.

[jhoffner]

BTW the only other thing that I can see that changed since moving to WebPack/Vue is that the node server now uses the connect-history-api-fallback package right before it tells express to use the static public folder. It's possible that is somehow messing with files being loaded correctly.

[kazk]

In this case, I deployed to staging and it wiped out production's assets, and once the cached files expired on their servers the assets were no longer available to be retrieved again.

😨

---

WebPack is nice but really complicated :( I'm also interested to know what happened. Can you post the config?

By the way, since you have source maps enabled, I was able to see many of the .vue files. I don't know if this was intentional, but I liked what I saw :)

[jhoffner]

Good catch on the source maps. I'm not too worried about it but I'll take them out of the next build.

Here is the config. Its all pretty standard stuff that came with Vue, except for a few loaders. https://gist.github.com/jhoffner/a3635cea60e7e01482d7856fe02ed30e

[kazk]

@jhoffner
I'm guessing the chunk hash of the manifest didn't change when the contents changed (webpack's issue 4253).
Wouldn't this explain the weirdness? I guess this combined with Cloudflare's CDN will eventually cause mismatching states pointing to non-existent resources.
If this was the issue, inlining the manifest seems be the workaround (jouni-kantola/inline-chunk-manifest-html-webpack-plugin).

---

Some other random thoughts and notes:

Hint: In combination with long term caching you may need to use the ChunkManifestWebpackPlugin to avoid that the vendor chunk changes. You should also use records to ensure stable module ids.
https://webpack.js.org/plugins/commons-chunk-plugin

https://webpack.js.org/guides/caching/

---

Add minChunks: Infinity?

https://webpack.js.org/plugins/commons-chunk-plugin/#combining-implicit-common-vendor-chunks-and-manifest-file

// split vendor js into its own file
new webpack.optimize.CommonsChunkPlugin({
  name: 'vendor',
  minChunks: function (module, count) {
    // any required modules inside node_modules are extracted to vendor
    return (
      module.resource &&
      /\.js$/.test(module.resource) &&
      module.resource.indexOf(
        path.join(__dirname, '../node_modules')
      ) === 0
    )
  }
}),
// extract webpack runtime and module manifest to its own file in order to
// prevent vendor hash from being updated whenever app bundle is updated
new webpack.optimize.CommonsChunkPlugin({
  name: 'manifest',
  chunks: ['vendor'],
  minChunks: Infinity
}),

[jhoffner]

These are some really good finds. Thanks for this. I was starting to wonder if the chunks were actually updating or not. The odd thing though is its often that the manifest itself doesn't load, even though I verified that the manifest attempted to being loaded was in fact within the dist directory.

I'm going to dig into this more in a bit.

[kazk]

@jhoffner are you trying to inline now? I noticed Kumite not loading and found

    window.webpackManifest = {
      "0": "static/js/0.03848b231fff6ad163d2.js",
      "1": "static/js/1.d41ac71755e853213f69.js",
      "2": "static/js/2.33838d7e686f5ee6f8da.js",
      "3": "static/js/3.2646ba21e87879fdfc24.js"
    }

If runner.codewars.com is using vuejs-templates/webpack, maybe their issue 406 "use HashedModuleIdsPlugin to keep module ids consistent" is related?

---

Some notes:

To generate identifiers that are preserved over builds, webpack supplies the NamedModulesPlugin (recommended for development) and HashedModuleIdsPlugin (recommended for production).
https://webpack.js.org/guides/caching/#deterministic-hashes

---

Use an approach to avoid that module ids change between compilations. This means either use records, NamedModulesPlugin or HashedModuleIdsPlugin. Each one has disadvantages. I would recommend records if your infrastructure allow it.
https://gist.github.com/sokra/ff1b0290282bfa2c037bdb6dcca1a7aa

---

webpack... 😵