Add test exclusion list

[mtrezza]

New Feature / Enhancement Checklist

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.

Current Limitation

We are planning to move Parse Server database adapters into their own repositories, out of the Parse Server repository. The goal is to reduce the weight of the repository and reducing vulnerabilities for developers who do not user certain adapters. It also allows to develop additional DB adapters without having to touch the Parse Server repository.

The CI of a separate DB adapter repository should run all Parse Server tests, using the specific DB adapter. Since not every DB adapter supports all DB related tests, we need to a mechanism to exclude specific tests. This is currently done on the server side with only_db or exlude_db. With external adapters, the adapter itself needs to define which tests to exclude. This also helps when developing new adapters.

Feature / Enhancement Description

Overload the jasmine it test method to add an ID to the test, and skip the test if that ID is in an exclusion list. On Parse Server, this exclusion list will be empty, but in the external adapter repo this exclusion list can contain all the IDs of tests that the DB adapter does not support (yet).

As ID a UUIDv4 must be used to ensure uniqueness; no hand typed strings.

Example Use Case

Jasmine test method overload in helper.js:

// Fetch test exclusion list
const testExclusionList = require('./testExclusionList.json');

// Overload test method
global.it_id = id => {
  if (testExclusionList.includes(id)) {
    return xit;
  } else {
    return it;
  }
};

Test defintion:

it_id('bbd9e2f6-7f61-458f-98f2-4a563586cd8d')('example test', async () => {
});

Alternatives / Workarounds

• Exclude a test by it's jasmine path (describe.describe.it) or test name, so that an additional ID doesn't need to be introduced --> this is not a robust solution as tests may be restructured or renamed which would break all adapter tests.

[parse-github-assistant]

Thanks for opening this issue!

• 🎉 We are excited about your ideas for improvement!

[ddrechse]

Hi,

So where I get confused is where this fits in with how CI is done for Adapter repos. Is Parse Server src copied and then a specific adapter and then build and test?

The code that adds the exclusion id, is that run once in tests in server/spec directory?
And the the updated specs are merged into server repo?

Thnx

[mtrezza]

So where I get confused is where this fits in with how CI is done for Adapter repos. Is Parse Server src copied and then a specific adapter and then build and test?

The source of tests is the Parse Server repo. The adapter repo merely runs them. Since an adapter may not support all features, the adapter repo needs to be able to exclude certain tests. The exclude mechanism needs to be implemented on the Parse Server repo, the adapter repo merely specifies a list of excluded test IDs.

Running the Parse Server tests in the adapter repo means not copying some files and executing them. Unfortunately, the Parse Server test setup is way to complex to do this, with a lot of mocking and hacking going on in various helper files. Also the relevant tests are not confined to specific files, so we most likely need to run the full test suite. That means probably the adapter repo needs to clone(?) the Parse Server repo, set its adapter as the DB adapter to use in tests and then run all the tests.

[ddrechse]

if the adapter repo clones server repo and runs the tests then why isn't the current mechanism sufficient
it_exclude_dbs(['oracle'])('geo max distance medium', async () => {

I think this should still work, yes?
global.it_exclude_dbs = excluded => {
if (excluded.indexOf(process.env.PARSE_SERVER_TEST_DB) >= 0) {
return xit;
} else {
return it;
}

I like that format because its human readable and I can tell quickly what dbs don't work for that test

[mtrezza]

it_exclude_dbs(['oracle'])

This exclusion is part of the test definition. There are 2 types of tests:

• a) Parse Server tests that are defined in the parse server repo. These tests cannot have an exclusion, in their definition because Parse Server cannot know (and doesn't care) which tests an adapter supports and which ones it doesn't support and therefore need to be excluded.
• b) DB adapter specific tests that are defined in the adapter repo. These are primarily meta tests that cannot be part of the Parse Server tests because they have nothing to do with Parse Server or any other DB adapter. These should be only a few tests. Since they are run by the adapter repo - in addition to the Parse Server tests - they don't require an it_exclude_dbs because they only exist in the adapter repo.

In the long run, we will abandon the it_exclude_dbs, because all DB adapters will move out of Parse Server into their own repos (except maybe the MongoDB adapter which may remain as the default adapter).

[ddrechse]

Thnx Lets get back to the first part of this change, generating a unique id for each test.

Is this generation run once in the parse server repo?
If a new test is added, and checked into parse server repo, how does that test get a unique identifier? Is that incumbent on the user checking it in? or is there a tool?

[mtrezza]

Since jasmine doesn't have a UID feature we need to implement it ourselves.

Options for UID:

• a) use test name --> not robust, name can change or have duplicates
• b) use describe path + test name --> not robust, names can change and describe nesting too
• c) manually create a UID and add it to the test definition

So (c) is what we've concluded.

In theory every test should have a UID, but in practice only tests that are excluded need a UID to be identified in the exclusion mechanism. The mechanism is described in the top post. So we'll make this an incremental change and add UIDs for:

• tests that have a it_only_db(mongo) and it_exclude_db(mongo), these will require a UID because they are either irrelevant for the Oracle adapter or only relevant for Postgres.
• tests for features that you know already the Oracle DB adapter does not yet support at this point

The manual generated UID will be a UUIDv4, so anyone can generate them easily for example using an online service (search for "UUIDv4 generator"). The mechanism also needs a check whether a UID is used multiple times, which can happen if tests are copied and the contributor forgets to change the UID.

I guess in theory it would be possible to write a script and add a UID to tests automatically.

[ddrechse]

Super thanks

Here's what I intend to do
I'm going to work in Oracle Storage Adapter tree cause I can run CI

I am going to add UUIDs for the tests specified above and fix up helper.js to implement exclusion lists

Whats the expected format of the exclusion list? JSON Array format?

Test it and then we'll see what's what
Is good?

[mtrezza]

Just to make sure we're talking about the same; that exclusion mechanism and the IDs would be added to Parse Server. So you intend to open a PR on this Parse Sever repository to add the mechanism and the UIDs?

In the end we would need to have a PR against this public repository's alpha branch. The PR needs to be from your feature branch that is based on the unmodified alpha branch of your clone of this repository. Otherwise we can't merge the PR as it will contain other changes that are unrelated to the exclusion mechanism, like Oracle adapter related changes.

[ddrechse]

Yup, I think we agree.
I will test it out in my current Oracle Storage Adapter branch
If that works, I will clone alpha, add the exclusion list changes and push it
Is good?

[mtrezza]

Sounds great! Thanks for picking this up.

[ddrechse]

So that worked well, I just took the changes from above and stuck them in helper.js
I created a testExclusionList.json
[
"bbd9e2f6-7f61-458f-98f2-4a563586cd8d",
"e1e86b38-b8a4-4109-8330-a324fe628e0c",
"05f1a454-56b1-4f2e-908e-408a9222cbae",
"9ee376ad-dd6c-4c17-ad28-c7899a4411f1",
"9e35a89e-bc2c-4ec5-b25a-8d1890a55233",
"6df434b0-142d-4302-bbc6-a6ec5a9d9c68",
"d9fbc5c6-f767-47d6-bb44-3858eb9df15a",
"3ec537bd-839a-4c93-a48b-b4a249820074",
"0a248e11-3598-480a-9ab5-8a0b259258e4",
"f52379e7-4f44-4fa8-bd3e-f0accdc6292d",
"e65d5f91-ad09-40c5-9020-c691a49f02a9",
"8adc2b43-871c-4871-8123-cf75bde77beb",
"0b16bee9-5cb5-4002-959f-7bcc938fb128",
"aaaf1c2b-e53e-4d1f-9435-748a9b6a7194",
"0b073d31-0d41-41e7-bd60-f636ffb759dc",
"26c9a13d-3d71-452e-a91c-9a4589be021c"
]
Added these to the GeoPoint test suite for the tests I didn't want to run
it_id('e65d5f91-ad09-40c5-9020-c691a49f02a9')('invalid input withinPolygon', done => {
And it all worked as expected

I can pull the latest Alpha add my changes and push the branch
Is OK?

[mtrezza]

Amazing! Yes please go ahead. You'd pull the alpha branch, then add your changes in a new feature branch based on the alpha branch. From that feature branch you'd then open the PR.