Test suite for MagicMirror²

[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.

1. Check keys in the translation files. Produces errors currently, so I’ve added .skip.
   https://github.com/qistoph/MagicMirror/commit/123392c54934e49a397d586c1fb8dbcc4cc5d12b

2. 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;
				}
			}

[roramirez]

@qistoph Nice work. Related with your first point there a discussion about translations of JSON file.
https://github.com/MichMich/MagicMirror/pull/679