rewrite :readme-examples from spek to junit5

[robstoll]

Platform (all, jvm, js): all Extension (none, kotlin 1.3): none

Code related feature

the readme-example project executes tests and write the output of the failure into the README.md of Atrium. It is based on Spek and since Spek doesn't advance, has bugs which makes it impossible for Atrium to upgrade respectively, we should move away from spek entirely. This includes also the readme-example project. Instead of providing an own TestEngine (which uses SpekTestEngine) we should write just normal tests and use a TestExecutionListener or similar (something which is able to turn a failing test into a successful one and vice versa) to achieve the same.

This isn't a good first issue but might be fun to tackle. In the end, executing ./gradlew :readme-example:build should generate the same output in README.md (see for instance <ex-first> in README.md and the corresponding FirstExampleSpec

Once we don't rely on spek, we can also update junit https://github.com/robstoll/atrium/pull/1677

[hubtanaka]

Hello @robstoll, can I work on this?

write just normal tests and use a TestExecutionListener

I'm thinking to try this literally in first.

[robstoll]

@hubtanaka sure go for it. Now that you are part of the core contributors, you can assign an issue to yourself by your own 🙂

I don't know if one can turn a failing test into a successful one with a TestExecutionListener. So that's something to figure out.

[hubtanaka]

Since last Monday, my daytime job has suddenly become busy💤 So I finally started to work on this issue.

I'll read some recent commits related to Spek.

[hubtanaka]

As conclusion of this weekend, TestExecutionListener seems not to be able to change test results for me.

What I did

• added MyJUnitTest.kt and MyJUnitExecutionListener under readme.examples.
• MyJUnitTest includes some simple JUnit tests.
• MyJUnitExecutionListener implements TestExecutionListener and override executionFinished.
• if testExecutionResult?.status is FAILED, executionFinished call a private function named handleFailed.
• handleFailed calls this.executionFinished(testIdentifier, TestExecutionResult.successful()), which is similar approach to handleFailure in ReadmeExecutionListener.
• but this method does not change test results.
• (commit) https://github.com/robstoll/atrium/compare/main...hubtanaka:atrium:survey/change-test-result-with-TestExecutionListener

So I did not find a simple way to handle failing test with TestExecutionListener.

I'm thinking to find another way to solve this.

[hubtanaka]

I'm reading https://junit.org/junit5/docs/current/user-guide/ According to the docs, DynamicTest (TestFactory) may be able to change test results, I've never used it though. I don't know if it's possible, but I'll try it.

[robstoll]

I suppose an InvocationInterceptor could do the job (did not try it out tough)

[hubtanaka]

@robstoll thanks for your information! I'll read the document and try it.

[hubtanaka]

I suppose an InvocationInterceptor could do the job (did not try it out tough)

You are right. It worked 💯

I wrote two simple test classes and two extensions which override InvocationInterceptor. https://github.com/robstoll/atrium/compare/main...hubtanaka:atrium:survey/change-test-result-with-InvocationInterceptor

ex1) MyExtendWithInvocationInterceptorTest with MyNeverFailExtension -> the result of assertFalse is turned into passed

ex2) MyExtendWithInvocationInterceptorForEachTest with MyNeverFailExtension and MyNeverPassExtension
-> the result of failedToPassed is turned into passed, and the result of passedToFailed is turned into failed.

So as next step, I'll try to implement an InvocationInterceptor which has handleSuccsess and handleFailure like ReadmeExecutionListener.

[hubtanaka]

I would like to write an interim report.

What I did

• copied readme-examples package and named it readme-examples-junit5
  • added gradle task: readme-examples-junit5
  • wrote all readme-examples with junit5
  • copied ReadmeTestEngine and named it JUnitReadmeTestEngine
  • this package and the content files are temporary. In the end of this issue, these files will be deleted and become diffs of the files under readme-examples
• current results
  • Task :readme-examples-junit5:readme

    • 80 tests successful

    • has problems that task fails for each successful run with FileAlreadyExistsException.

      • PathSpec, which I rewrote, seems not to delete toDelete.
      • deletePaths needs to be invoked explicitly in JUnitReadmeTestEngine.execute, maybe.
      • -> I'll try this later.
  • README.md
    • At first glance, it looks almost good.
    • stack trace seems longer than before: ex-toThrow1, ex-notToThrow, ex-third-party-10
• branch: https://github.com/robstoll/atrium/compare/main...hubtanaka:atrium:survey/ReadmeTestEngine

This issue has become a long journey a little bit, but I think I'll be able to create a PR in a few more days.

[robstoll]

@hubtanaka first of all, don't stress yourself, totally fine if it takes longer, regardless of the reason. Maybe it is harder/trickier than expected, you have other work to do or just because it is nice outside and you want enjoy the good weather. Your contribution is very much appreciated regardless the time it needs :slightly_smiling_face:

I'll probably have time to take a look at your branch next week

ps: Since spek is not well supported by intellij (by now). Following a hint, how you can run a single spec (might come in handy to debug a problem). Since the spec is under src/main intellij doesn't provide a run gutter or run in the context menu. Annotate the spec with org.junit.platform.commons.annotation.Testable. Following an example: This way there is the run gutter and you can run/debug one spec at a time.

[hubtanaka]

@robstoll thank you for your kindness and information. yeah, this issue might be difficult a little bit for me. however, I still think I can do something.

I'll try your spek test tech later. it seems useful. Thanks a lot😊

[robstoll]

Finally had time to look at your branch. The problem you see with the deletion comes from two things:

• PathSpec deletes the temp directory in the deletePaths annotated with @AfterAll
• Your custom TestEngine doesn't respect @AfterAll in the sense of it never calls it

=> due to this the folder is already there once you call PathSpec a second time.

A simple workaround is to move the deletion into the method:

  @Test
    fun `ex-path-symlink-and-parent-not-folder`() {
        val tmpdir = Paths.get(System.getProperty("java.io.tmpdir")).resolve("atrium-path")
        try {
            val directory = Files.createDirectory(tmpdir)
            val file = Files.createFile(directory.resolve("file"))
            val filePointer = Files.createSymbolicLink(directory.resolve("directory"), file)

            expect(filePointer.resolve("subfolder/file")).toBeARegularFile()
        }finally {
            tmpdir.deleteRecursively()
        }
    }

However, this only hides that the root cause is the custom engine (and would include the workaround in the readme). I suggest we don't follow the custom engine approach as it brings several problems:

1. we need to be fully compatible with the junit API => we would need to use again a delegation approach as we did with Spek, then it is easier as the underlying testEngine already (should) make suer to follow the JUnit API (spek did not, therefore we also had problems in the past)
2. We would need to use an own Annotation because @Test would also be run by JunitJupiter (and with an own annotation IDE support would be less nice).

So I took a look at InvocationInterceptor again and came up with a solution which is still a bit a hack but works better overall as it uses Jupiter and no custom Engine: https://github.com/robstoll/atrium/pull/1834

[hubtanaka]

Thanks for your hard work.

However, this only hides that the root cause is the custom engine (and would include the workaround in the readme). I suggest we don't follow the custom engine approach as it brings several problems:

I agree. To be honest, I felt my custom engine approach was overkill for our purpose.

I'll review your PR (and I'd like to learn how to use InvocationInterceptor)