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

Procedures test workspace cleanup #4311

Merged
merged 4 commits into from Sep 28, 2020

Conversation

moniika
Copy link
Contributor

@moniika moniika commented Sep 24, 2020

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Closes #4080

Proposed Changes

  • Refactors tests to generate separate tests for procedures_callreturn and procedures_callnoreturn rather than use this.callForAllTypes.
  • Adds try/finally blocks with workspaceTeardown where appropriate
  • Removes unnecessary block.dispose calls (handled in Workspace.dispose)
  • Removes unnecessary deleteVariable calls (each test has its own Workspace)

Reason for Changes

  • Without separating logic into separate test calls, if test assert failed for procedures_callnoreturn (was run first) then test was not run for procedures_callreturn. This makes it easier to debug if there is a bug specific to procedures_callreturnorprocedures_callnoreturn`
  • Workspaces are now always disposed even if assert fails
  • Removing unnecessary logic helps simplify tests

Test Coverage

Ran mocha tests

Tested on:

  • Desktop Chrome

Documentation

n/a

Additional Information

Behavior of tests unchanged.

@moniika moniika changed the title Procedure test ws cleanup Procedures test workspace cleanup Sep 24, 2020
tests/mocha/procedures_test.js Outdated Show resolved Hide resolved
@moniika moniika merged commit 3b36497 into google:develop Sep 28, 2020
@moniika moniika deleted the procedure-test-ws-cleanup branch September 28, 2020 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants