Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test attempts to run despite module resolution failure #69

Closed
keturn opened this issue Sep 27, 2021 · 2 comments
Closed

test attempts to run despite module resolution failure #69

keturn opened this issue Sep 27, 2021 · 2 comments
Labels
Type: Bug Issues reporting and PRs fixing problems

Comments

@keturn
Copy link
Contributor

keturn commented Sep 27, 2021

I had some tests that failed with unhelpful error messages. Among all the noise in the logs, I found a "ModuleRegistry has no latest version for module" message, but MTE was trying to forge ahead regardless.

It made for a confusing situation when there hadn't been a resolution error halting the test, yet the things I expected from that module weren't loaded.

@keturn keturn added the Type: Bug Issues reporting and PRs fixing problems label Sep 27, 2021
@keturn
Copy link
Contributor Author

keturn commented Sep 27, 2021

This is almost a reversal from the situation we had previously, where we had too many dependency resolution errors blocking tests.

A lot of what we've done to clean that situation up has been reducing the number of methods that attempt to do module resolution. Ideally that's the responsibility of ModuleManager and nobody else should get involved.

I bet I removed some module resolution stuff that was happening earlier in the process, something that did show a higher-profile error message. With that seemingly redundant check gone, the error case presents in a different way.

Probably that StateHeadlessSetup#createGameManifest should outright fail if it can't make a manifest that contains all those modules, instead of just logging about it.

If we change it there (rather than in the MTE-specific subclass), it will affect the headless server too. I think that's probably fine and good but it's worth checking how headless servers are actually configured.

That would be an engine change, but there's probably work to do on the MTE side too to make sure an error there is presented in a useful way in the test results.

@keturn
Copy link
Contributor Author

keturn commented Jun 1, 2022

@keturn keturn closed this as completed Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues reporting and PRs fixing problems
Projects
None yet
Development

No branches or pull requests

1 participant