Skip to content

fix: correct launchUrl typo and promote demo test from xfail#3778

Open
Kunall7890 wants to merge 1 commit into
sandstorm-io:devfrom
Kunall7890:fix/demo-login-launch-url-v2
Open

fix: correct launchUrl typo and promote demo test from xfail#3778
Kunall7890 wants to merge 1 commit into
sandstorm-io:devfrom
Kunall7890:fix/demo-login-launch-url-v2

Conversation

@Kunall7890

Copy link
Copy Markdown

Problem

Fixes #3615

commands/loginDemo.js had a typo on the first .url() call:

// ❌ Before
.url(this.launchUrl + "/demo")

this.launchUrl does not exist in Nightwatch — the correct property
is this.launch_url (snake_case). This caused the browser to navigate
to undefined/demo instead of the actual server URL, making the demo
login test fail immediately on every run.

Changes

tests/commands/loginDemo.js

  • Fixed typo: this.launchUrlthis.launch_url

tests/tests/basic.js

  • Removed the if (run_xfail) guard around Test demo login command
  • The test is now a regular test alongside the other two
  • Removed unused run_xfail import

How to Verify

RUN_XFAIL=true ./run-tests.sh

Test demo login command should now pass instead of failing
on the first navigation step.

Fixes sandstorm-io#3615

- Fix typo in loginDemo.js: this.launchUrl -> this.launch_url
  (this.launchUrl was undefined, causing navigation to fail)
- Remove xfail guard from basic.js now that the test is fixed
- Remove unused run_xfail import from basic.js
@ocdtrekkie ocdtrekkie added the sandstorm-dev Issues hacking on Sandstorm label Jun 6, 2026
@ocdtrekkie ocdtrekkie linked an issue Jun 6, 2026 that may be closed by this pull request
@ocdtrekkie

Copy link
Copy Markdown
Collaborator

The test didn't pass in CI. I'm actually guessing though that could be because of #3765.

@Kunall7890

Copy link
Copy Markdown
Author

Hi @ocdtrekkie — thanks for the review!

Understood on #3765 potentially causing the CI failure. Since this is a
pre-existing build issue unrelated to my changes, would you be able to
confirm whether the fix itself looks correct to you?

Happy to rebase or make any other adjustments needed on my end.

@ocdtrekkie

Copy link
Copy Markdown
Collaborator

@mnutt Michael, how do you think we can test this?

@mnutt

mnutt commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

@Kunall7890 it looks like Sandstorm changed the copy from "Demo" to "Create Account" back in 2015. I made a small change here: mnutt@df12f950

If you want to pull it into your branch you could do:

git fetch https://github.com/mnutt/sandstorm.git mn/demo-login-launch-url-v2
git cherry-pick df12f950
git push

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

Labels

sandstorm-dev Issues hacking on Sandstorm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Demo tests in tests/basic.js are broken.

3 participants