Skip to content
This repository was archived by the owner on Apr 3, 2024. It is now read-only.

Move some tests out of test/standalone#214

Merged
kjin merged 5 commits intogoogleapis:masterfrom
kjin:standalone
Jan 7, 2017
Merged

Move some tests out of test/standalone#214
kjin merged 5 commits intogoogleapis:masterfrom
kjin:standalone

Conversation

@kjin
Copy link
Copy Markdown
Contributor

@kjin kjin commented Jan 6, 2017

Moved these tests out of test/standalone (with path changes and minor fixes to make them run independently of each other):

  • test-debuglet.js
  • test-module.js
  • test-options-credentials.js
  • test-sourcemapper.js

Also, moved test-v8debugapi.js back to test/standalone. Because it's always run last it didn't seem to have interference with other tests. But now that test-module.js is non-interference, test-v8debugapi.js fails unless being run alone.

All of the remaining standalone tests have to do with the v8debugapi object, which is a singleton. To make these tests standalone would involve adding a debug-only function to src/v8debugapi.js to modify the singleton... but was not sure if this would be a reasonable thing to do. Opinions wanted here.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 6, 2017
@ofrobots
Copy link
Copy Markdown
Contributor

ofrobots commented Jan 7, 2017

I'd be okay with adding a test-only API into src/v8debugapi.js to make the tests stateless. We don't export this file outside the module, so I don't have concerns about the misuse of the test-only API.

@kjin
Copy link
Copy Markdown
Contributor Author

kjin commented Jan 7, 2017

Sounds good, I will address that in another PR.

ofrobots
ofrobots approved these changes Jan 7, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@ofrobots ofrobots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if the tests are green.

@kjin kjin merged commit b5c2910 into googleapis:master Jan 7, 2017
@kjin
Copy link
Copy Markdown
Contributor Author

kjin commented Jan 9, 2017 •
edited
Loading

@ofrobots As it turns out, having tests being non-interfering also requires a change in index.js itself, to override the debuglet singleton (see here). Do you think I should go ahead with this, and if so, what would be a more suitable approach: exposing a test-only stop method, or enabling a test-only force flag that causes debuglet to be re-assigned when set? (I am leaning towards the latter.)

@ofrobots
Copy link
Copy Markdown
Contributor

ofrobots commented Jan 10, 2017

I like the idea of a test-only stopAgent method better, but I don't have really strong feelings on this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cristiancavalli cristiancavalli Awaiting requested review from cristiancavalli

1 more reviewer

@ofrobots ofrobots ofrobots approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kjin @ofrobots @googlebot