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

feat: handle namespaces when running tests #126

Merged
merged 6 commits into from
Feb 4, 2021
Merged

feat: handle namespaces when running tests #126

merged 6 commits into from
Feb 4, 2021

Conversation

AnanyaJha
Copy link
Contributor

@AnanyaJha AnanyaJha commented Feb 3, 2021

What does this PR do?

This PR adds support for namespaces when running tests - it addresses specifying a namespace ie. when working with a namespaced scratch org

What issues does this PR fix or reference?

@W-8725635@

@AnanyaJha AnanyaJha requested a review from a team as a code owner February 3, 2021 16:56
export class TestService {
public readonly connection: Connection;

constructor(connection: Connection) {
this.connection = connection;
}

// utils to build test run payloads that may contain namespaces
public async buildSyncPayload(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this functionality over to the library- we'll need these utils for the plugin and in VS Code. Consumers can still build their own payloads when calling runTestsAsync/Sync but these utils are helpful if there's a chance that namespace info may be present.

} as TestItem;
}

const namespaces = await this.queryNamespaces();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running the queries adds time to the operation so want to avoid querying unless absolutely necessary. Only query for namespaces if the test field is specified and it's a two part argument - myNamespace.myClass or myClass.myTest.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to be very inefficient because the query can potentially run on every item of the array. There's two ways in which I think we can address this.

The first, to split this into two different loops on the testNameArray. The first one to build the payload as complete as possible and split those items that might have a namespace into a separate structure. The second loop would consume the data structure generated for potential namespaces, run the namespace query once, continue processing the data structure and merge it with the one from the first loop.

The second, look at caching the results of queryNamespace during runtime so we only run the query once but we would still need to tweak when to run it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, I'll change this so we only query for the namespaces once during a test run rather than for each test/class in the test run

Comment on lines +67 to +69
detailed_code_cov_header: 'Apex Code Coverage for Test Run %s',
syncClassErr:
'Synchronous test runs can include test methods from only one Apex class. Omit the --synchronous flag or include tests from only one class'
Copy link

Choose a reason for hiding this comment

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

When are keys underscore vs camelcase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmmm good question, looking through right now and looks like we've got a bit of everything : - ) I'll update the ones in the library package for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I'll open a follow up for everything, it's going to get messy in this PR

namespaces = await this.queryNamespaces();
}

if (namespaces.has(testParts[0])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will error out if we run a namespaced tests against an org that returns no namespaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind it does work correctly. Can we add a test for that scenario ?

Copy link
Contributor Author

@AnanyaJha AnanyaJha Feb 4, 2021

Choose a reason for hiding this comment

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

Done - tests that the argument we build will treat the namespace as a classname & the class as a test method so eventually the test run would error out.

@AnanyaJha AnanyaJha merged commit b54fa7b into develop Feb 4, 2021
@AnanyaJha AnanyaJha deleted the aj/nsTest branch February 4, 2021 21:06
AnanyaJha added a commit that referenced this pull request Feb 9, 2021
AnanyaJha added a commit that referenced this pull request Feb 9, 2021
klewis-sfdc pushed a commit that referenced this pull request Oct 24, 2022
WillieRuemmele pushed a commit that referenced this pull request Jan 31, 2023
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.

3 participants