Test suite for MagicMirror²

qistoph

I agree, seeing MagicMirror become such a great success with lots of contributions and improvements is really nice. To keep the quality consistent and MagicMirror reliable with so many changes, it would really help to run tests automatically as much as possible.

Maybe I’ll be able to work on some test-cases too, so feel free to include me in any communication. I’m also on Trello, so would be able to join there too.

Good initiative!

roramirez

Thanks @qistoph

I know your help will to be great.

I recently a create a simple Trello board https://trello.com/b/RMlo1mDy/test-suite-magicmirror. Three state: Todo - Doing - Done

Todo: Ideas and planning of tests.
Doing: Task self asignated or requested one by someone. This state if someone is working in this task.
Done: Pushed in Pull Request or commited.

Every interested in help this area, feel free send me by IM or private message of your Trello account.

morozgrafix

Great idea to setup test suite. Thanks for getting it started @roramirez. I’ve made some minor cleanup changes on top of your initial work, PR is sitting on GitHub. I will try to contribute to tests as much as I can.

roramirez

@morozgrafix Thanks!, Is already merged :)

roramirez

Update and news:

Thanks to @qistoph we have 2 improvement

Now every commit introduced previous is checked by tests suite in Travis.
Added a new test case https://github.com/MichMich/MagicMirror/pull/648/files

Also, @qistoph and I suggest have two side for test.

The backend application and node_helpers
The frontend application running in electron, running client side javascript.

For this last is possible use Spectron … but… is needed will set nodeIntegration to true. One idea is check a enviroment variable and if present set this variable to true.

For other way, work with serveronly and get test from client side from there.

If some have comments and ideas from this is welcome.

PS: I have a sample of use spectron https://github.com/roramirez/MagicMirror/commit/c99581c15b4cb008b2800f40ecbaf9c9862c7ceb

qistoph

Okay, just another brainfart.

For the tests to run consistently and in such a way that we can test whatever is needed, we’re probably going to need a separate configuration file, maybe even a couple. If I’ve read the code correctly, we could set nodeIntegration in this config, without touching the code in MM to check for environment settings…

This would be cleaner imho

roramirez

@qistoph Sound good and I like it. Now we can set by electronOptions in configuration file, same logic as you say.

The point to resolved is how can tell to MagicMirror read a specific file of configuration. Now is set in hardcoded. With than can be create multiples instances of configuration files into tests suite (tests directory) and run test to these. Added this task in Trello.

roramirez

Hi,

Today I’ve working a problem of issue with Electron and other things, all details here
https://github.com/MichMich/MagicMirror/issues/577

After testing in two enviroment different for Lite and with Pixel on Raspbian, the process is long because you need the create SD and follow the instruction and check what could be the problem.

I think this can be automatic and running on a hosted system or my own machine. Someone know howto use a arm image for RaspberryPi and run like a Docker’s container? for run a test of enviroments. I think can be a help to include in the Test Suite.

morozgrafix

@roramirez I haven’t played with Docker in a while and never tried raspbian on a docker container. Quickly glanced at DockerHub and it seems that people are able to do that. https://hub.docker.com/search/?isAutomated=1&isOfficial=0&page=1&pullCount=0&q=raspbian&starCount=0
I may take a look into it, but it all depends on my availability.

roramirez

Hi, I recently pushed a Pull Request with two e2e testing.
https://github.com/MichMich/MagicMirror/pull/669

These use their own configurations like @qistoph mention and after added the MM_CONFIG_FILE variable for MagicMirror.

It’s really easy create new test for check a different part of MagicMirror and their modules.

strawberry 3.141

@roramirez what is the reason for a 10 sec timeout in each test? If there are a many tests this will sum up

roramirez

@strawberry-3.141 said in Test suite for MagicMirror²:

what is the reason for a 10 sec timeout in each test? If there are a many tests this will sum up

It’s does not accumulate. It’s a timeout, the maximun time for the test. You can set different timeout for every test.

morozgrafix

I haven’t looked thoroughly at the code and I wanted to suggest setting a default timeout that can be overwritten in individual tests as needed. It may make test code a little bit cleaner and less repetitive.

Good job getting it going!

roramirez

@morozgrafix Yes, I knowed is not more cleaner and repetitive but was the first proof of concept ;)

Good idea set default timeout. I added two new task in Trello board about you mentioned.

morozgrafix

@roramirez I’ve played a little with test suite and added basic test for clock module. Then I realized that if we want to test different config options for a given module we may need to have to create multiple configs, which may be challenging with current tests directory organization.
Also I believe that process.env.MM_CONFIG_FILE = "tests/confs/config_name.js" line needs to be moved into before step (seems to work there or it can go into beforeEach if needed). Otherwise when running npm run test:e2e first config that gets picked up seems to persist throughout all of the tests and always used by the app.js.

I was wondering if structure similar to this makes sense:

tests
├── configs
│   ├── env.js
│   └── modules
│   │   └── clock
│   │   │   └── clock_12hr.js
│   │   │   └── clock_24hr.js
│   │   │   └── clock_analog.js
│   │   └── helloworld
│   │       └── helloworld.js
├── e2e
│   ├── env_spec.js
│   └── modules
│       ├── clock_spec.js
│       └── helloworld_spec.js
├── functions
│   └── compare-version_spec.js
└── global_vars
    └── root_path.js

but this would involve changing app.use("/tests/confs", express.static(path.resolve(global.root_path + "/tests/confs"))); in the server.js to be somewhat dynamic and I’m not very familiar with express on how it can be done. I think that’s easily solved by just changing it to app.use("/tests/configs", express.static(path.resolve(global.root_path + "/tests/configs")));

roramirez

@morozgrafix Seem good sense going to a refactor of the structure.

I think we can take two way acord you mentioned.

1.- Include your test for clock module with new format for name and respective directories
2.- Do it the refactor to all tests that remain.

I really like see how you resolve the multiples instances configuration in a one tests file.

morozgrafix

@roramirez cool thanks. I have it all ready on my fork. Will submit PR for review.

qistoph

Two commits I’ve worked on for the testing.

Check keys in the translation files. Produces errors currently, so I’ve added .skip.
https://github.com/qistoph/MagicMirror/commit/123392c54934e49a397d586c1fb8dbcc4cc5d12b
To prevent loading app.js from corrupting the Mocha test environment, I suggest to execute the app.js in a virtual environment. This can also serve as an example for future test cases where code needs to be executed in the global namespace.
https://github.com/qistoph/MagicMirror/commit/cd8bee1371ffc6cce7b7bf44f85cd03705e4c1bd

Any thoughts before I submit a PR?

strawberry 3.141

@qistoph i like the idea of the sandbox

is there a way to just warn instead of error for missing translation keys?

qistoph

@strawberry-3.141
How about this?
https://github.com/qistoph/MagicMirror/commit/406ae4e8c37cbf7e31c89f5341d7715bacbcf0d2

			try {
				expect(fileKeys).to.deep.equal(baseKeys);
			} catch(e) {
				if (e instanceof chai.AssertionError) {
					this.skip();
				} else {
					throw e;
				}
			}