Introduced E2E Testing using TestCáfe

[sgraband]

To run the tests run `testcafe chrome browser-app/test/test.ts´ in the client folder.

yarn e2etest in the client folder starts theia and the tests.

Part of Issue #17

Signed-off-by: simonGraband [email protected]

[CamilleLetavernier]

To run the tests the path to the default Workspace needs to be set at the top of the file.

Absolute paths must absolutely be avoided, as it is not possible for each developer to adjust the path to his own local setup. We should maybe consider passing the workspace path programmatically, as an argument, when running the test (Something similar to the start:debug script, with --root=../workspace

Even localhost:3000 is probably an unreliable value, as it's just the default port on which the server is started, when nothing else is specified

[sgraband]

I understand the problem. But i am not quite sure how to handle it. Right now there seems to be no way to create custom parameters for testcafe tests. Do you have an idea on how we could work around that?

[tortmayr]

Maybe we could simply resolve the relative path "../../workspace" with the help of the path library? Similar to how the jarpath is resolved in the coffee editor:

https://github.com/eclipsesource/coffee-editor/blob/73db58f116cd75967a663d0a758545ef484031f9/web/coffee-server/src/node/backend-module.ts#L39

[sgraband]

I made the Path relative. However i am not quite sure how and if we should handle the problem with the port, that @CamilleLetavernier mentioned.

[eneufeld]

how would one start the tests? must this be added to the package.json ? can we pass the port there?

[eneufeld]

no don't touch the package.json, just adapt the code to check for the variable and otherwise read the value from the config file as shown in the SO link. In the config you set the port to 3000 .

[sgraband]

Ok i did that. I did not use the config file i just created a const in the test, that takes the enviroment variable and if that is not available just calls 3000.

[eneufeld]

Why not use the config file?

[sgraband]

The config file only works, if you call the run command in the directory the config is in, so in our case browser-app/test. This would prevent the developer from calling the tests in the client directory. If we are fine with that I can add it to the config file.

[eneufeld]

I don't see this in the documentation. Aren't you simply importing the config file via a path in your test?

[eneufeld]

Could you update the commit message by applying some of the tips from this blog.
So basically use imperative eg, Introduce E2E Testing using TestCáfe .
I think it would be beneficial to squash the two commits and adapt the commit message on what needs to be adapted to run the tests.

[eneufeld]

I think it makes sense to move the new test files from the browser-app folder to a new tests or e2e-tests.
Further add a .testcaferc.json and add a tsConfigPath property. The referenced tsconfig should also be part of the new folder. The tsconfig should be something like this:

{
    "compilerOptions": {
      "target": "es2015",
      "module": "commonjs",
      "moduleResolution": "node",
      "resolveJsonModule": true
    }
}

Ideally it should enable the strict property.
The .testcaferc.json would be something like this:

{
  "tsConfigPath": "./tests/tsconfig.json"
}

With this the config can be imported: import * as config from "./config.json"; .

Besides this refactorings and code improvements, the tests didn't work for me.
The relPathToWorkspace is not working for me, I needed to remove one '..'.
I also needed to change the fixture to fixture`Ecore-glsp E2E-Testing`// declare the fixture .page`http://localhost:${port}/#${relPathToWorkspace} .

In order to ease the test execution I suggest to add an entry to the package.json in the client folder which would start theia and start the tests.

[eneufeld]

It might make sense to squash all commits into one after the requested changes.

[sgraband]

@eneufeld What would be the best way to start theia from the client folder package.json? The way that i do it is by cd browser-app and then yarn start. Is there an easier way to add this to the scripts?

[eneufeld]

@simongraband I would do it just as you suggest

[sgraband]

@eneufeld Ok i tried to do that, however since the yarn start never stops, i cannot run the two commands sequentially. After searching i found npm-run-all which allows to run the commands parrallel. This works, but now all log-messages of the yarn start are displayed in between the test-feedback.

Do you know any way to supress the log messages of yarn start? All the solutions i found do not work...

[eneufeld]

Sorry I don't know any option. Did you try the different options described here: https://github.com/mysticatea/npm-run-all/blob/master/docs/npm-run-all.md ?

[sgraband]

I think the best option would be --aggregate-output. With that argument the results of the tests is supressed until the end and shows them all at once. In my opinion this is not a perfect but acceptable solution. What do you think?

[sgraband]

Commits are now squashed.

[eneufeld]

@CamilleLetavernier could you recheck
@simongraband this needs to be rebased

[CamilleLetavernier]

The changes look really good! A few more comments:

• Currently, each developer needs to install testcafe. I would add it to the dev-dependencies of the project, so it would be installed automatically
• Since the Ecore GLSP Server also has to be started separately, I would add a short section about how to run the tests in the README (Start the Ecore server, then run yarn..)

Now I can run the tests and a lot of things happen, but I still get 13 failures out of 18 tests

[sgraband]

I couldn´t add testcafe to the dependencies using npm install --save-dev testcafe. So i did it manually. Could you quickly check if this is working?

Regarding the 13 failures. I realized, that there is currently a bug, that does not update the workbench view, when the filesystem is updated. It only works after reloading. This leads to the failures in the tests. Also there are some tests which are failing since the tested feature is not working properly.

[tortmayr]

FYI: since we are using yarn you can install devdependencies with yarn add --dev but adding them manually should work just fine as well

[sgraband]

Ok so i found some problems with the tests and fixed them. Now only 5/18 tests should fail. Those are

• Change Attributetype (due to Bug in Type Editing via Keyboard #89)
• Delete Edges (due to Fix inheritance implementation #103)
• Delete Edges + Delete Nodes (due to inconsistent eraser behavior)
• Move Edges (due to inconsistent routing of edges)

@CamilleLetavernier could you quickly confirm that these are the errors you encounter as well?
How should we handle the Testcases that fail due to inconsistent behavior. Should we comment them out or leave them in?

[sgraband]

@eneufeld seems like the python bug appeared again. Could you maybe re-run the build again?

[CamilleLetavernier]

I initially had 9/18 failures, but 4 of them are related to a dirty workspace (Containing uncommitted ecore models). Maybe we should consider using a different workspace specifically for the tests to avoid this; but this can be ignored for now.

For reference, the tests that fail on a dirty workspace are:

 ✖ Open Workbench

   1) AssertionError: expected 9 to deeply equal 4

 ✖ Deletion/Renaming of enotation

   1) AssertionError: expected 9 to deeply equal 4

✖ Create and Delete ecore file

   1) - Error in test.after hook -
      AssertionError: Check if glsp-graph.enotation exists: expected false
      to be truthy

 ✖ Create Nodes

   1) - Error in test.after hook -
      AssertionError: Check if glsp-graph.enotation exists: expected false
      to be truthy

After running a full clean/rebuild, I still have 6 failures (Or 5 in the first run):

 ✖ Delete Nodes with Eraser

   1) AssertionError: Class has been deleted: expected true to be falsy

 ✖ Delete Nodes with ENTF

   1) The specified selector does not match any element in the DOM tree.

✖ Move Edges

   1) AssertionError: expected 67.41666666666667 to deeply equal
      62.41666666666667

✖ Delete Edges

   1) AssertionError: Reference deleted: expected true to be falsy

3 of these errors seem to be expected; I'm not sure about the Delete Nodes with ENTF though.

 ✖ Delete Edges with ENTF

   1) AssertionError: Inheritance deleted: expected true to be falsy

May be expected as well (But ENTF should probably be renamed to DEL :) )

 ✖ Renaming Classes/Attributes

   1) AssertionError: Renamed Abstract: expected false to be truthy

Is this an unexpected failure?

Also, one of the expected failures seems to work for me:

 ✓ Change Attributetype

Even though #89 is now fixed on master, the fix isn't included in this branch, so I'm not sure if that's really expected.

In general, the waiting times are a bit long. It took 5~6 minutes to run the tests, because many assertions have a 5 seconds wait time. It's not always easy to avoid wait times completely, so let's ignore this for now. We can investigate a better solution later on.

[CamilleLetavernier]

Blocking issues have been solved; I think it's OK to merge this now. Follow-up tickets should be created for the remaining issues (When such tickets don't exist already)

[sgraband]

@CamilleLetavernier regarding your testresults:

The Fail with the with ENTF-deletion(i will refactor that :)) has a ticket(#103) and is expected.

The only tests that do not behave like expected are Change Attributename and Renaming Classes/Attribute. I am not quite sure why Change Attributetype works for you. And for the Renaming Classes/Attribute i am not sure either. But it could be a runtime error, where the test is unable to find the Object in time, because the auto-layout didn´t happen for some reason.

[CamilleLetavernier]

Should the tests be updated to match the recent changes?

I get errors such as:

✓ Delete Nodes with DEL
 ✖ Create Edges

   1) The specified selector does not match any element in the DOM tree.

       > | Selector('div.tool-group > div.tool-button')
         |   .withText('Composition')

But PR #105 changed the label "Composition" to "Containment". There may be other outdated tests, as I now have 8/18 failures

[sgraband]

I fixed the tests so that only the expected tests are failing.

• Delete Edges (due to Fix inheritance implementation #103)
• Delete Edges (due to inconsistent eraser behavior)
• Delete Nodes (due to inconsistent eraser behavior)
• Move Edges (due to inconsistent routing of edges)

Also updated the README, since the server, client and tests can now be started with one command.

[CamilleLetavernier]

Awesome! :)

[eneufeld]

I would like to solve the issue with the build before merging this, maybe the whole issue is solved when we update theia to the latest version which uses node 10, I talk with @tortmayr

[eneufeld]

@simongraband I talked with @tortmayr and we are sure the build issue would be solved with an update of glsp and theia
In the meantime I suggest to downgrade the node version used in travis to 8 as this should resolve the build issue, but please add an issue or a comment to the glsp update issue to update node again

[sgraband]

@eneufeld do you have a link to the issue?

Also node 8 seems not to work...

[eneufeld]

#109

[eneufeld]

weird that node 8 is not working, that used to work, I clean the cache ..

[sgraband]

@eneufeld I rebased this branch with the changes of #110 the build still seems to fail. Locally i can run it with Node 10

[eneufeld]

Looks like it was a cache issue