bugfix: run tests with directAccess

[dblythy]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

Closes: #8808

Approach

Tasks

• [ ] Add tests
• [ ] Add changes to documentation (guides, repository pages, code comments)
• [ ] Add security check
• [ ] Add new Parse Error codes to Parse JS SDK

[parse-github-assistant[bot]]

🚀 Thanks for opening this pull request!

[dblythy]

Just a note that I tried to remove Parse.CoreManager.setRESTController(RESTController); entirely and it broke a number of tests

[dplewis]

That’s how you know it works, we can figure out what’s not supported with direct access: true

[dblythy]

Should we:

• Change directAccess to false in helper.js or
• Fix all the tests to pass installationId / directAccess: false where required

[dplewis]

Change directAccess to false in helper.js

We could have a separate CI job with PARSE_SERVER_ENABLE_EXPERIMENTAL_DIRECT_ACCESS=1 and slowly chip away at it.

Fix all the tests to pass installationId / directAccess: false where required

Ideally pass all tests. We can always go back to the directAccess: false tests and add support if possible.

@mtrezza Your call

[mtrezza]

We should parameterize the entire test suite. Similar to data providers in PHPUnit. That allows us to easily run directAccess with true and false across the all tests. This should be implemented in the test setup, not on an it or describe level. In the future (or even now) we may have other options that we want to add, maybe even permutate. This will significantly increase robustness of our CI. It could be useful to add an option to disable or override these parameters via the CI, for example if we want an additional run with directAcess: true only on Node 22 and exclude all other Node versions to save resources. As for the transition, we could exclude parametrization on a per-files basis. That may be useful to further optimize if some files should be permanently excluded from running with multiple options.

[dplewis]

We should parameterize the entire test suite.

We already have this with the configuration options in helper.js. For example we could set directAccess: false as default but if the environment variable is set it will turn true across the entire suite.

if we want an additional run with directAcess: true only on Node 22 and exclude all other Node versions to save resources.

If we do this are you ok with the build failing until all tests pass?

[mtrezza]

Good if we already have some of the logic in place. I think it needs to be refactored a bit to make this parameterization feature clearer. It also may need handling of reconfigureServer in a test that tries to override the param, probably throw an error, as the param is managed by the test suite.

Regarding failing workflow for directAcess: true, we could set it as non-required in the GitHub CI. That's not very elegant for reviewers. I think the param logic that we need to build will likely benefit from a per-file exclusion. Some test files just won't need to run multiple times with different param values. That way we can gradually include files that support directAccess: true without having the CI failing.

[dblythy]

What is the path forward for this PR?

[mtrezza]

I believe we need a logic in the test suite that allows to:

• define a list of Parse Server options and their values to run all tests on
• exclude variations on a per-file basis, where it's unnecessary to run multiple tests
• throw error if a test tries to set one of these vars with configureServer as they will be managed by test suite

A suggestion:

const options = [
  key: 'directAccess',
  values: [
    true,
    false
  ],
  // an optional list regex expression that allows to include or exclude files based on their filenames, default should be '.*' which runs on all tests
  files: [
   '.*'
  ]
];

[dblythy]

As it stands there are a number of tests that fail with direct access on (they either need installationId or they aren't compatible at all) - how do you suggest approaching this?

[mtrezza]

I think we could set the files key to exclude files that contain tests that do not pass during the transition period. I believe there are no tests that cannot pass because the feature they are testing fundamentally doesn't work with directAccess: true, but if there are any - and they would probably only be a handful - they'll go into a separate file that will be permanently excluded.

[dblythy]

I'm not sure if it's possible to access the current spec file from within helper.js. I tried to log global.currentSpec from within the reconfigureServer function and it is null. Also I'm not sure if running all the test twice (in direct access and then without) is the best idea, considering our test times are already quite long

[mtrezza]

I'm not sure if it's possible to access the current spec file from within helper.js

Why are you trying to do that?

I'm not sure if running all the test twice (in direct access and then without) is the best idea,

How else can we know whether directAccess does not break anything? Like the recently reported bug.