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

Remove $$PREBID_GLOBAL$$ from unit tests #1508

Closed
dbemiller opened this issue Aug 21, 2017 · 2 comments
Closed

Remove $$PREBID_GLOBAL$$ from unit tests #1508

dbemiller opened this issue Aug 21, 2017 · 2 comments
Labels

Comments

@dbemiller
Copy link
Contributor

The $$PREBID_GLOBAL$$ references are causing a few problems in our test code.

First, they're fragile. #1437 was logged about this, and in the time it took to fix the existing references, several more PRs got merged which broke them again.

Second, they cause weird behavior. #1427 surfaced an issue where a test passed when using config.setConfig, but failed when using $$PREBID_GLOBAL$$.setConfig--even though they're the same function. I traced the root problem to the use of window. Currently, our webpack preprocessor is making a new bundle of Prebid for every unit test file... but the window object (and therefore $$PREBID_GLOBAL$$) persists across files. This is a very subtle bug which makes writing good unit tests tricky.

Lastly, they prevent us from improving our unit test performance. Due to the window issue, many tests surrounding $$PREBID_GLOBAL$$ fail if we do change the way we bundle the code for testing. It's unfortunate, because the unit tests would only take ~15s to complete (rather than the ~45s they take currently). This difference will only increase as we add new unit test files.

We should refactor our code to be more testable, and remove all $$PREBID_GLOBAL$$ references from the existing tests.

@dbemiller
Copy link
Contributor Author

Fixing this will be a marathon, not a sprint.

My proposal for working towards a solution is outlined in #1510. By encapsulating $$PREBID_GLOBAL$$ state into smaller objects, and injecting those objects as dependencies throughout most of the code, we will gradually work towards a codebase which does not rely on $$PREBID_GLOBAL$$ for testing.

@stale
Copy link

stale bot commented Mar 6, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 6, 2018
@stale stale bot closed this as completed Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant