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

Java System.out.print is affecting Kata tests. Bad solution can be submitted. #867

Closed
dinglemouse2250 opened this issue Mar 22, 2017 · 12 comments

Comments

@dinglemouse2250
Copy link

dinglemouse2250 commented Mar 22, 2017

A user encountered some weird problem running one of my Kata.

  • His solution fails my test cases
  • He included some Java System.out.print statements and magigically the Solution "passes". Actually it still fails same as before but CW does not recognize it anymore as failing!

--

I have reproduced his problem and the System.out.print REALLY DO affect the test result in some strange way.

Futhermore, if I change his System.out.print statements to println instead of print then the bad Kata solution fails again like it was ought to...

Something weird going on...

PSA screenshot showing the Output window when using System.out.print.

  • Notice the Passed number (1) is plain wrong.
  • Notice the error message shows on the screen but CW still thinks the solutions is a pass.

cw-print-issue

FYI - Here is link to his solution code:
https://www.codewars.com/kata/reviews/56f53a5b6dc4feeaa2000044/groups/58d1e2f7f4080cf53d0000bb

@kazk
Copy link
Member

kazk commented Mar 22, 2017

@jhoffner maybe you figured this out already, but this is probably caused by the following:

System.out.print:

user logged output<FAILED::>

System.out.println:

user logged output
<FAILED::>

@kazk
Copy link
Member

kazk commented Mar 22, 2017

Confirmed this reproducible in multiple languages. If the assertion failure doesn't write to stderr, and there's at least one <PASSED::> recognized, the test is considered passed.
I think this is essentially the same as codewars/codewars-runner-cli#127 where the user's output interferes with test result.


JavaScript behaves differently when the tests are not inside describe.

function add(a, b) {
  if (a != 1) return a + b;
  process.stdout.write(''+a);
  return a - b;
}

with describe

describe("Solution", function() {
  it("should test for something", function() {
    Test.assertEquals(add(1, 1), 2);
    Test.assertEquals(add(2, 1), 3);
  });
});

image

without describe

Test.assertEquals(add(1, 1), 2);
Test.assertEquals(add(2, 1), 3);

image

@blarosen95
Copy link

If I refactor my submission, will you all still have access to the bug related code?

@jhoffner
Copy link
Member

The best way to handle this across all languages would be to either go through all of them and ensure that <PASSED::> and <FAILED::> are on their own line (by prefixing with \n), or updating the output parser to allow those tokens to exist anywhere in the line.

The later would be easier to make the change but I'm not certain of what edge cases that could create.

@kazk
Copy link
Member

kazk commented Mar 22, 2017

@jhoffner
How are blank lines handled? I tried in Kumite and it seems to be completely ignored.

console.log(`
a


b
`); // including the leading and the trailing

image

I don't know if this is intentional, but prefixing with \n looks safe.

Changing the output parser might be easier in short term, but I think it should be avoided because of the uncertainty. I also like the idea of "each new STDOUT line is considered a new piece of data" :)

@jhoffner
Copy link
Member

@kazk I agree.

This change (for Java only) will be fixed within the next Coderunner deployment.

codewars/codewars-runner-cli#336

@kazk
Copy link
Member

kazk commented Mar 22, 2017

I can start changing other languages if you'd like.
Updating lib/runners/X.js and test/runners/X_spec.js and opening a PR per language?

@kazk
Copy link
Member

kazk commented Mar 22, 2017

@blarosen95 I think every distinct solution gets a permalink so no need to worry. Also the cause of this issue is found :)

@blarosen95
Copy link

I figured you guys had the issue pinpointed 👍

@jhoffner
Copy link
Member

Changes are in preview, www is deploying now.

@kazk sounds good. Thanks!

kazk added a commit to kazk/codewars-runner-cli that referenced this issue Mar 25, 2017
kazk added a commit to kazk/codewars-runner-cli that referenced this issue Mar 25, 2017
kazk added a commit to kazk/codewars-runner-cli that referenced this issue Mar 26, 2017
kazk added a commit to kazk/codewars-runner-cli that referenced this issue Apr 1, 2017
kazk added a commit to kazk/codewars-runner-cli that referenced this issue Apr 4, 2017
kazk added a commit to kazk/codewars-runner-cli that referenced this issue Apr 25, 2017
kazk added a commit to kazk/codewars-runner-cli that referenced this issue May 3, 2017
kazk added a commit to kazk/codewars-runner-cli that referenced this issue Jun 7, 2017
kazk added a commit to kazk/codewars-runner-cli that referenced this issue Jul 14, 2017
jhoffner pushed a commit to codewars/codewars-runner-cli that referenced this issue Jul 17, 2017
artemabalmasov added a commit to artemabalmasov/codewars-runner that referenced this issue Sep 27, 2017
* Groovyserv POC

* Java docker fixes and groovyserv warm-up

* make this PR buildable

* [Crystal] Prepend LF to output commands

* [Elixir] Prepend LF to output commands

* [PHP] Prepend LF to output commands

* [PHP] format php_spec.js

* eslint fixes

* travis img configuration

* consistent usage of template string

* Java config in the Makefile

* Updated readme

Minor typos

* Make Travis pass

* Add basic BF support

* Add test integration for BF

* Set the number of allowed cells to 30000

The number 30000 comes from the original interpreter.
`bf` defaults to 9999 and the manual says "Usually you need less."
Adjust this later if this is too large.

* Add BF documentation

* Add BF examples

* Lock ESLint version to 3.19.0

ESLint 4.0.0 was released on 2017-06-11 and introduced breaking changes.
Use ESLint 3.19.0 for now.

 - http://eslint.org/blog/2017/06/eslint-v4.0.0-released

* no longer replaces double spaces with single

* - Java: Custom package/class names now work (one top level class per file limit is still in affect)
- Dart: locked version to 1.23.0-1 to fix docker file build error

* an offering to the eslint gods

* Fixed Java “setup” code

* java spec improvements

* Docker file for Chapel added

* Chapel envivonment information added

* basic Chapel example

* custom Chapel test framework added (cw-2)

* first class functions do not work in error-handling methods

* Chapel runner added

* improved sanitized Chapel error output

* Chapel setup added

* Tests for Chapel added

* Added Chapel in Makefile

* Add Chapel to Travis

* [Chapel] fixed indentation for ESLint

* Added ham crest-core jar to Java

* added opts.dir back in for groovyclient

* an offering to the ESLint god

* ordered list of installed packages

* Added Node8 support + Mocha/Node versions

- Added Node8 and updated Node 6 (LTS) to v6.11.0 (with legacy support for specifying 6.6.0)
- Switched to manually starting the Mocha global binary so we can swap Node versions with Mocha now, too
- The JS runner now throws a useful error if provided an invalid version
- Added a series of specs to ensure we are running the version of Node we expect

* Updated docs based on code review

* Support for julia 0.6

* update travis.yml

* Fixes Crystal by installing a specific version

- Thanks to kazk for getting the majority of the work done on this.
- I had to add `shards` to the path binaries to get it to build.
- The `crystal` inside `embedded/bin` is not the same, and could not run the code. Very weird.

* Update Makefile

* Increased Chapel's timeout

* Update julia_spec.js

* Update julia.js

* fixed

* update julia.js

* Update julia.js

* Added Gradle/Spring support to Java
- GroovyServ is no longer used. Gradle’s daemon is used instead. Unfortunately performance is about 800ms slower on avg.
- Ability to customize which packages get loaded
- Spring support
- Ability to code blocks into multiple files (all languages)
- Multi-file Java support
- Improved Java exception output
- Java build info is now presented within output

* - removed groovyserv reference
- code cleanup

* Offering to the ESLint gods

* more eslint fixes

* Removed groovyserv install

* Prefer outputFileSync over codeWriteSync

* OMG ESLint leave me the hell alone!

* Enabled Typescript in Karma & Angular 2

- TypeScript can now be run using `karma_tdd` and `karma_bdd` with Mocha
- Angular 4 can be tested (with quite a bit of included code)
- Split out some of the Karma logic so it can be reused in the TS runner. Since it's still highly specialized, I didn't move it to a shared module yet.
- Upgraded Angular 1.5 and added Angular 1.6 to the mix for now
- Increased the default timeout for running TS tests (it takes at least 6-7 seconds to run Typescript+Karma)

* Pinned the versions of Typings, Coffeescript, and TypeScript

- Also reordered them to keep the two TS libraries together.
- ESLinting

* Rewrite Swift runner

Rewrite swift runner using `XCTestObservation` protocol.
Test results are now reported through `CodewarsObserver` which is
registered by `_XCTMain`.

This way, we no longer need to hack the source.

- Use Swift version 3.1.1, official build for Ubuntu 14.04
- Files are now compiled with `swiftc` and are no longer concatenated
  - This passes existing tests, but requires further testing
  - `solutionOnly`:
    - `opts.solution` is written to `main.swift`
    - `opts.setup` is compiled together if provided as `setup.swift`
  - `testIntegration`:
    - `opts.fixture` is written to `main.swift`
    - `opts.solution` is compiled together as `solution.swift`
    - `opts.setup` is compiled together if provided as `setup.swift`
- Make Travis build and test swift-runner
- Remove cw-2 test framework
- Update examples

Required Changes

- Use `_XCTMain` instead of `XCTMain`
- `import XCTest` is required

Output Compatibility

Outputs are mostly compatible with the original.
The failure messages are now reported with `<FAILED::>description` not
`<FAILED::>Test Failed` followed by `<ERROR::>description`.
The output for test cases with multiple assertions changed.
`IT` only contains single `PASSED`, `FAILED` or `ERROR`.

Closes codewars#448

* Fix Test.assert(Not)ApproxEquals (codewars#435)

* Add relevant tests for fixing codewars/codewars.com#962
* Treat 1e-9 as absolute error margin when (un)expected value small enough

* Code Cleanup (codewars#452)

- docker images no longer use timeout as default entrypoint
- ESLint fixes

* Improve Travis build time (codewars#423)

* Improve Travis build time
* Remove Docker upgrade
Travis upgraded Docker to 17.03.1 which supports `--cache-from`.

* [F#] Prepend LF to ouput commands (codewars#419)

* Listen.js fix/Readme (codewars#453)

- docker images no longer use timeout as default entrypoint
- ESLint fixes
- listen.js no longer fails
- updated Readme to include BF and Chapel

* Ruby/BF/Chapel Updates (codewars#458)

* Ruby/BF/Chapel Updates
- Ruby now has bcrypt gem, for better Rails integration
- BF now supports setup code
- Chapel setup code was reworked to not use mutation

* Fix/warnings stderr (codewars#459)

* Java Compiler Warnings
- Compiler warnings have been moved into Build Output

* UTF8 Fix for Java (codewars#460)

* Disable warning on Tab usage (codewars#461)

* Fix Swift compatibility issue around XCTMain (codewars#465)

* Fix Swift compatibility issue around XCTMain

* Set Rust version to 1.15.1 (codewars#464)

* Set Rust version to 1.15.1

* Use rustup for better versioning

* Avoid a warning during rustup installation

    warning: $HOME differs from euid-obtained home directory: you may be using sudo

* [Nim] Add Nim

* [Nim] Add test integration

Implement CodewarsOutputFormatter and make use of the new custom test
report formatter feature.  We need to use the latest Nim since this was
merged few days ago.  Change back to a release tarball when a version
with this feature becomes available.

* [Nim] Add basic documentation

Update this when the version is set.

* [Nim] Prepend LF to output commands

codewars/codewars.com#867

* [Nim] Add docker-compose services

* [Nim] Fix ESLint errors

* [Nim] Fix CodeFactor issues

* [Nim] Add Nim to Travis

* [Nim] Set Nim version to 0.17.0

* [Nim] Add setup file support

* [Nim] Add test for <ERROR::>

* Update README

[ci skip]

* [OCaml] Prepend LF to output commands

* [Nim] Remove boilerplate

Remove the boilerplate in tests by utilizing `include` and coordinating tests.

    import unittest, codewars/formatter
    addOutputFormatter(OutputFormatter(newCodewarsOutputFormatter()))
    import solution
    include fixture

Unlike concatenating, error messages points to the correct location.

Tests can now be written like:

    suite "simpler tests":
      test "addition":
        check(add(1, 1) == 2)

* Add R support

R 3.4.1, test integration with testthat.
No other packages are installed for now.

- <https://www.r-project.org/>
- <https://github.com/hadley/testthat>

* [Go] Copy only necessary files

* [Lua] Copy only necessary files

* [Nim] Copy only necessary files

* [Swift] Copy only necessary files

* [Rust] Copy only necessary files

* [C/C++] Copy only necessary files

* [OCaml] Copy only necessary files

* [Python] Copy only necessary files

* [Ruby/Shell/SQL] Copy only necessary files

Combined gem install to avoid "max depth exceeded" error.

* [ObjC] Copy only necessary files

* [JS/TS/Coffee] Copy only necessary files

* [Julia] Copy only necessary files

* [Haskell] Copy only necessary files

* [Dart] Copy only necessary files

* [PHP] Copy only necessary files

* [Crystal] Copy only necessary files

* [BF] Copy only necessary files

* [Chapel] Copy only necessary files

* [Java] Copy only necessary files

* [JVM] Copy only necessary files

* [C#/F#] Copy only necessary files

* Unfreeze Erlang/Elixir

Lock Erlang OTP 18.3, Elixir v1.2.4

Supporting multiple version might be possible using asdf.

- <https://github.com/asdf-vm/asdf>

* [R] Copy only necessary files

* [TypeScript] Make targets configurable

Make `tsc` `--target` configurable through `opts.languageVersion`.
This does not affect testing with Karma since it's set to ES5.

* Make Elixir independent

Elixir is locked to 1.2.4 for existing contents.
Making Elixir independent allows us to add newer versions of Erlang.

* Add Erlang support

Erlang/OTP 20.0, test integration with EUnit

Note that tests without `desc` are named "Unnamed Test".
This is because:

- Generating a name from `source` and `id` can result in unexpected
  names
- Generating a name from the "parent" group is difficult because some
  macros produces groups without `desc`

Also note that groups without `desc` are ignored in the output because
it's hard to differentiate from groups generated by some macros.

* Rewrite rust runner

Features

- Use Cargo
  - Install `rand` package
- Improve output
  - Align with other languages
  - Include stdout of failed tests
  - Proper termination

Other Changes

- Include examples to tests
- Restructure tests by context

Structure

Needs some hack in order to maintain backward compatibility and avoid errors with duplicate import.

    .
    |- Cargo.toml
    `- src
       |- lib.rs    (opt.setup + opts.solution + module declaration)
       `- tests.rs  (opts.fixture)

- Placing `opts.fixture` in submodule `tests.rs` should avoid issues with duplicate imports
- Maintains backward compatibility by prepending `use super::*;` to `opts.fixture` if missing

Another idea was

    .
    |- Cargo.toml
    `- src
       |- lib.rs      (module declarations)
       |- solution.rs (opts.setup + opts.solution)
       `- tests.rs    (opts.fixture)

- Maintaining backward compatibility is hard
  - Requires solutions to be declared public
  - Requires tests to import solution `use solution::*;`

Using `rand` in tests

    extern crate rand
    use self::rand::Rng;

Needs `self::` because `extern crate` loads to current name-space and tests are in `tests` module.

* Fix OCaml

- Lock versions
- Use external solver aspucd
- Remove wrapper shell script

* Download AngularJS files from code.angularjs.org

Reduce the size of build context by ~12 MB.
The list of files was generated by:

    # in frameworks/javascript/angular
    ls **/*.js | sed -e 's_^_https://code.angularjs.org/_;s/$/ \\/'

* Adds PowerShell language support

* Adds example tests for PowerShell

* Fix PowerShell

- Fix ESLint errors
- Cleanup
- Copy only necessary files
- Run tests at the end of build

* stdout

* Hide props from output

* Java now supports proper DESCRIBE/IT with total COMPLETEDIN time

* added ms to Java COMPLETEDIN output

* Add Kotlin support

Kotlin 1.1.3-2

Test Frameworks
- JUnit 4 <http://junit.org/junit4/>
- KotlinTest <https://github.com/kotlintest/kotlintest>

Assertion Libraries
- kotlin.test <https://kotlinlang.org/api/latest/kotlin.test/kotlin.test>
- Kluent <https://github.com/MarkusAmshove/Kluent>
- Expekt <https://github.com/winterbe/expekt>

* Add Groovy support

Groovy 2.4.12

Test Frameworks
- JUnit 4 <http://junit.org/junit4/>
- Spock <http://spockframework.org>

* Add Scala support

Scala 2.12.3

Test Frameworks
- JUnit 4 <http://junit.org/junit4/>
- ScalaTest <http://www.scalatest.org/>

* Fix contaminated builds

Fix the issue where the code from previous builds are included.

- Remove `--project-cache-dir` which is used to store project specific
  information like incremental build
- Use subdirectory "project"
- Use `--offline` to build without accessing network resources
- Exclude tasks for other languages
- Update `build.gradle`
  - Use `implementation` instead of deprecated `compile`

* Remove Kotlin/Scala installations from jvm-runner

* Reduce memory usage and simplify

- Explicitly set memory limits to reduce memory usage
- Update Gradle to 4.1
- Remove pointless `GRADLE_DAEMON_FLAG` (not visible from `docker exec`)
- Use `prewarm.sh` to start Gradle daemon
- Always use `--daemon`, this will only start new daemon process if not
  already running or exiting process is busy

* Add Groovy/Kotlin/Scala documentation

* Add tests and examples for Spock

* Add JUnit tests and examples for Scala

* Add algorithms example for KotlinTest

* Remove R installation from alt-runner

* Add typings/ back

* [Rust] Allow `non_snake_case`

Disable this warning for existing contents.

* Lock mongoose version to 4.10.x

`DeprecationWarning` in `mongoose >= 4.11.0` breaks existing content.

* [SQL] Silence Sequel deprecation warning

* [JS] Prepend LF to output commands (cw-2)

* [JS/TS] Prepend LF to output commands (mocha)

Fix LF issue for Mocha.
Move 'mocha-reporter' into this repository.

* Upgrade Kotlin to 1.1.4

* Reduce `util.codeWriteSync` usage

TypeScript
Destination was changed to `opts.dir` ('/home/codewarrior') for clarity.
Passing `codeDir` of `null` defaults to `/home/codewarrior`.

* Remove lib/copydir.js

* Remove `util.exec`

* Split lib/util.js

* Remove mkdirParentSync

* Remove unused parameter from writeFileSync

No caller calls with overwrite false.

* Add "use strict" and improve consistency

* Remove `shovel.CompileError`

Only used by `lib/runners/javascript.js` when Babel throws and has no
special treatment.

* Throw Error instead of strings

* Split test/runners/javascript_spec.js

* Make csharp_spec.js readable

* Rename lib/options.js

* Call track() when requiring 'temp'

Recommended in
<https://www.npmjs.com/package/temp#want-cleanup-make-sure-you-ask-for-it>

Eventually switch to `opts.dir`.

* Refactor runners

- Decouple runners from lib/shovel.js
- Change the signature of shovel to introduce createRunner(strategies)
- Remove customStrategy
  - codewars#396 (comment)
- Make transition to promise-based smooth

* Move JavaScript runner

* Split JavaScript runner

* Move Ruby runner

* Split Ruby runner

* [Swift] Fix invalid reference `test.framework`

* Add Solidity runner

Truffle v3.4.8 (core: 3.4.8)
Solidity v0.4.15 (solc-js)

* - added ethereum and enzyme packages to node
- added eventide to ruby

* updated ruby from 2.3.0 to 2.3.3

* Replace listen.js

* reverted to ruby 2.3.0 since there are build errors with latest versions (2.3.3 and 2.4.1)

* Ethereum for Node
- JS compiled example contracts
- added web3 and related packages for working with ethereum APIs

* uncommented out the test cases

* jslint fixes

* - Ethereum specs no longer timeout
- improved contract generation

* Added zeppelin support, a critical library to building contracts

* new timeout for karma based tests are 20s

* Refactor objc.docker

Reduce uncompressed size from 2.94 GB to 1.93 GB

* Gradle Optimization Improvements
- when-prewarmed utility added to ensure we wait until the starting daemon is ready
- cleaned up test cases
- improved test case prewarm integration
- increased jvm memory

* Spec fixes
- added gradle.properties back in
- when-prewarmed eslint fix

* clean no longer expects done

* Fix to enable running TS/Karma with a version

- Now the `languageVersion` is not passed into the Karma/Node runner code, so we don't get Node version errors.
- Updated Node docs to mention Karma testing on TS
@ma-za-kpe
Copy link

Screenshot from 2019-05-09 22-32-20
log info not being printed.

@kazk
Copy link
Member

kazk commented May 9, 2019

@ma-za-kpe that's because you're only allocating char[] and not putting anything in it. So your code is basically doing System.out.println((char)0);. The output is shown like that because it's not a printable character.

Do char[] n = str.toCharArray(); or copy the chars into n.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants