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: test faucet website #702

Open
wants to merge 6 commits into
base: next
Choose a base branch
from

Conversation

TomasArrachea
Copy link
Collaborator

@TomasArrachea TomasArrachea commented Feb 18, 2025

Closes #673

This PR adds a test for the faucet website. It uses fantoccini as a browser client to test that all components are loading correctly.

To run fantoccini it needs a webdriver running. I'm using chromedriver here, which requires to have Chrome installed.

Also, this PR introduces a stub for the node RPC API. This allows to run the faucet and test it using the stub, instead of having to run the full node.

@TomasArrachea TomasArrachea force-pushed the tomasarrachea-test-faucet-website branch from dad20af to 801aa22 Compare February 20, 2025 17:57
Comment on lines +281 to +284
let mut chromedriver = Command::new("chromedriver")
.arg(format!("--port={chromedriver_port}"))
.spawn()
.expect("failed to start chromedriver");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to install Chromedriver and Chrome to run this locally, but I am not sure on why it's running fine in the CI

Copy link
Contributor

Choose a reason for hiding this comment

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

The github CI images have a bunch of pre-installed stuff (I believe the image is like 50GB). This means less cpu wasted installing things etc. And given the amount of webdev, probably almost any semi-popular tooling for it will be pre-installed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we create a target in the makefile to install Chromedriver and Chrome?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you can unless you assume the OS? Its a pity the dep doesn't have a bundling option, makes this sort of thing much easier.

@TomasArrachea TomasArrachea marked this pull request as ready for review February 20, 2025 17:58
@TomasArrachea TomasArrachea changed the title WIP: test faucet website feat: test faucet website Feb 20, 2025
Copy link
Contributor

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

Looks good & interesting (not something I'm familiar with), thank you.

I've left some initial questions and suggestions; I think we may also want to organise the test files a bit somehow but we can address that after everything else.

Right now this tests that the website loads and has the correct title. Would we then extend this with requesting funds from the faucet as well? (separate PR etc maybe).

Comment on lines 238 to 241
.ok_or(anyhow::anyhow!(
"Couldn't get any socket addrs for endpoint: {}",
config.endpoint
))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.ok_or(anyhow::anyhow!(
"Couldn't get any socket addrs for endpoint: {}",
config.endpoint
))?;
.with_context(|| format!("no sockets available on {}, config.endpoint))?;

Copy link
Contributor

Choose a reason for hiding this comment

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

Though I'm unsure under what circumstances that would even occur.

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

Successfully merging this pull request may close these issues.

3 participants