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

fix(learn): Cross browser testing - correct misleading text #31092

Merged
merged 8 commits into from
Feb 16, 2024

Conversation

ShamarYarde
Copy link
Contributor

@ShamarYarde ShamarYarde commented Dec 18, 2023

Previous text implied that driver.executeScript("lambda-status=passed"); and driver.executeScript("lambda-status=failed"); should be ran inside a Bash terminal shell instead of inside added inside the lambdatest_google_test.js demo code.

Running either as a command in a Bash terminal shell results in the following:

bash: syntax error near unexpected token `"lambda-status=passed"'

if

driver.executeScript("lambda-status=passed");

is ran, and

bash: syntax error near unexpected token `"lambda-status=failed"'

if

driver.executeScript("lambda-status=failed");

is ran.

Adding either within the lambdatest_google_test.js demo code produces respective results on the LambdaTest Automation dashboard. Also, it looks like these should be inside if and else blocks of code.

Description

Changed explanatory text to indicate where code should be placed.

Motivation

To allow readers to understand how to mark a test an automation test's status as passed or failed. Also, to remove confusion on where to look to make changes in the Your own remote server section.

Additional details

Related issues and pull requests

"Fixes #31074"

Previous text implied that `driver.executeScript("lambda-status=passed");` and `driver.executeScript("lambda-status=failed");` should be ran inside a bash terminal shell instead of inside added inside the `lambdatest_google_test.js` demo code.
@ShamarYarde ShamarYarde requested a review from a team as a code owner December 18, 2023 03:54
@ShamarYarde ShamarYarde requested review from pepelsbey and removed request for a team December 18, 2023 03:54
@github-actions github-actions bot added the Content:Learn:Cross-Browser-Testing Learning area Cross-Browser-Testing docs label Dec 18, 2023
Copy link
Contributor

github-actions bot commented Dec 18, 2023

Preview URLs

External URLs (1)

URL: /en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Your_own_automation_environment
Title: Setting up your own test automation environment

(comment last updated: 2024-02-16 01:42:52)

@@ -485,15 +483,15 @@ Let's write an example:

When executing numerous automation tests, marking their status as passed or failed makes the task a lot easier.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd propose putting these in the example above so that there is no doubt about where they go. Then delete this section and add an explanation below step 5 in the previous section about what this code does.

But perhaps do it in the other PR so that we're only working on one PR per document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it here, because the refactored code and updated tutorial text to reflect changes was merged by the time I got back.

Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Dec 18, 2023
Copy link
Contributor Author

@ShamarYarde ShamarYarde left a comment

Choose a reason for hiding this comment

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

Lines of code moved to lambdatest_google_test.js example to remove confusion.

ShamarYarde and others added 2 commits December 18, 2023 13:13
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added merge conflicts 🚧 [PR only] and removed merge conflicts 🚧 [PR only] labels Dec 18, 2023
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@ShamarYarde
Copy link
Contributor Author

@hamishwillee I ran out of minutes, so I'm not able to fix the sauce_google_test.js example in the Filling in Sauce Labs test details programmatically section, but it looks like it's missing the username in the saucelabs' updateJob method call and the driver.sessionID is undefined.

Previous text implied there was more than one code block in the `google_test.js` example.
@dipikabh dipikabh changed the title corrected misleading text fix(learn): Cross browser testing - correct misleading text Jan 16, 2024
@bsmth bsmth requested review from hamishwillee and removed request for pepelsbey January 24, 2024 13:45
@github-actions github-actions bot added size/s [PR only] 6-50 LoC changed and removed merge conflicts 🚧 [PR only] labels Feb 16, 2024
@hamishwillee
Copy link
Collaborator

@ShamarYarde Sorry for the delay - this wasn't initially assigned to me, and I missed the notification when it was. Will look now.

…_own_automation_environment/index.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@hamishwillee
Copy link
Collaborator

@ShamarYarde So what you've done is moved the stuff about marking tests as passed or failed from the separate section Filling in test details on LambdaTest programmatically into the body of the main example. Reading that deleted section assumed you'd update the example code, not enter in bash.

That said, I think doing it this way is actually reasonable - there isn't a particular need to highlight this option separately in its own section - in fact by putting it in the body of the example you could argue it makes this something that people are more likely to do by default.

@hamishwillee
Copy link
Collaborator

I ran out of minutes, so I'm not able to fix the sauce_google_test.js example in the Filling in Sauce Labs test details programmatically section, but it looks like it's missing the username in the saucelabs' updateJob method call and the driver.sessionID is undefined.

@ShamarYarde I'm going to merge this PR. Do you think you now have bandwidth to look at this case ^^^

Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Thanks very much!!!

@hamishwillee hamishwillee merged commit 047f999 into mdn:main Feb 16, 2024
9 checks passed
@ShamarYarde
Copy link
Contributor Author

I ran out of minutes, so I'm not able to fix the sauce_google_test.js example in the Filling in Sauce Labs test details programmatically section, but it looks like it's missing the username in the saucelabs' updateJob method call and the driver.sessionID is undefined.

@ShamarYarde I'm going to merge this PR. Do you think you now have bandwidth to look at this case ^^^

@hamishwillee No, I'm still out of minutes. I don't have a subscription.

@hamishwillee
Copy link
Collaborator

Thanks @ShamarYarde - I've created an issue to track the missing bit in #32401 - does that look about right?

As an aside though

  • you know the user name needs to exist in the updateJob method, so wouldn't it be best to update the docs indicating some value, even if we can't verify it? I.e. no worse than it is now.
  • You say "driver.sessionID is undefined" - can you expand on what you think needs to happen in the docs?

@ShamarYarde
Copy link
Contributor Author

ShamarYarde commented Feb 22, 2024

@hamishwillee I don't remember what I saw when I tried it, but it wasn't working. I just looked at the code I have, and the following lines were commented out which means that they were causing the error I was getting:

saucelabs.updateJob(username, get_session_id(), {
      name: "Google search results page title test",
      passed: testPassed,
})

The response when I run the command: node sauce_google_test.js now is the following:

/.../selenium-test/node_modules/selenium-webdriver/lib/error.js:558
        throw new ctor(message)
              ^

WebDriverError: Uh oh, you've run out of minutes!
Please visit https://saucelabs.com/pricing to purchase a subscription.
    at Object.checkLegacyResponse (/.../selenium-test/node_modules/selenium-webdriver/lib/error.js:558:15)
    at parseHttpResponse (/.../selenium-test/node_modules/selenium-webdriver/lib/http.js:593:13)
    at Executor.execute (/.../selenium-test/node_modules/selenium-webdriver/lib/http.js:529:28)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  remoteStacktrace: ''
}

Node.js v20.11.0

@hamishwillee
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Learn:Cross-Browser-Testing Learning area Cross-Browser-Testing docs size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filling in test details on LambdaTest programmatically Section Confusion
2 participants