-
Notifications
You must be signed in to change notification settings - Fork 13.5k
ci cleanup: rustdoc-gui-test now installs browser-ui-test #143851
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
use std::error::Error; | ||
use std::path::{Path, PathBuf}; | ||
use std::process::Command; | ||
use std::{fs, io}; | ||
|
||
/// Install an exact package version, and return the path of `node_modules`. | ||
pub fn install_one( | ||
out_dir: &Path, | ||
pkg_name: &str, | ||
pkg_version: &str, | ||
) -> Result<PathBuf, io::Error> { | ||
let nm_path = out_dir.join("node_modules"); | ||
let _ = fs::create_dir(&nm_path); | ||
let mut child = Command::new("npm") | ||
.arg("install") | ||
.arg("--audit=false") | ||
.arg("--fund=false") | ||
.arg(format!("{pkg_name}@{pkg_version}")) | ||
.current_dir(out_dir) | ||
.spawn()?; | ||
let exit_status = child.wait()?; | ||
if !exit_status.success() { | ||
eprintln!("npm install did not exit successfully"); | ||
return Err(io::Error::other(Box::<dyn Error + Send + Sync>::from(format!( | ||
"npm install returned exit code {exit_status}" | ||
)))); | ||
} | ||
Ok(nm_path) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,8 +76,6 @@ COPY scripts/nodejs.sh /scripts/ | |
RUN sh /scripts/nodejs.sh /node | ||
ENV PATH="/node/bin:${PATH}" | ||
|
||
COPY host-x86_64/x86_64-gnu-tools/browser-ui-test.version /tmp/ | ||
|
||
ENV RUST_CONFIGURE_ARGS \ | ||
--build=x86_64-unknown-linux-gnu \ | ||
--save-toolstates=/tmp/toolstate/toolstates.json \ | ||
|
@@ -91,15 +89,6 @@ ENV HOST_TARGET x86_64-unknown-linux-gnu | |
|
||
COPY scripts/shared.sh /scripts/ | ||
|
||
# For now, we need to use `--unsafe-perm=true` to go around an issue when npm tries | ||
# to create a new folder. For reference: | ||
# https://github.com/puppeteer/puppeteer/issues/375 | ||
# | ||
# We also specify the version in case we need to update it to go around cache limitations. | ||
# | ||
# The `browser-ui-test.version` file is also used by bootstrap to emit warnings in case | ||
# the local version of the package is different than the one used by the CI. | ||
ENV SCRIPT /tmp/checktools.sh ../x.py && \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can now delete the comment above, it's outdated. The version file itself can also be deleted, and rustdoc-gui-test can just hardcode the version for now, later we'll switch it to Btw the same outdated comment is also in |
||
npm install browser-ui-test@$(head -n 1 /tmp/browser-ui-test.version) --unsafe-perm=true && \ | ||
python3 ../x.py check compiletest --set build.compiletest-use-stage0-libtest=true && \ | ||
python3 ../x.py test tests/rustdoc-gui --stage 2 --test-args "'--jobs 1'" |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,61 +1,17 @@ | ||
use std::path::{Path, PathBuf}; | ||
use std::process::Command; | ||
use std::sync::Arc; | ||
use std::{env, fs}; | ||
use std::{env, fs, io}; | ||
|
||
use build_helper::npm; | ||
use build_helper::util::try_run; | ||
use compiletest::directives::TestProps; | ||
use config::Config; | ||
|
||
mod config; | ||
|
||
fn get_browser_ui_test_version_inner(npm: &Path, global: bool) -> Option<String> { | ||
let mut command = Command::new(&npm); | ||
command.arg("list").arg("--parseable").arg("--long").arg("--depth=0"); | ||
if global { | ||
command.arg("--global"); | ||
} | ||
let lines = match command.output() { | ||
Ok(output) => String::from_utf8_lossy(&output.stdout).into_owned(), | ||
Err(e) => { | ||
eprintln!( | ||
"path to npm can be wrong, provided path: {npm:?}. Try to set npm path \ | ||
in bootstrap.toml in [build.npm]", | ||
); | ||
panic!("{:?}", e) | ||
} | ||
}; | ||
lines | ||
.lines() | ||
.find_map(|l| l.rsplit(':').next()?.strip_prefix("browser-ui-test@")) | ||
.map(|v| v.to_owned()) | ||
} | ||
|
||
fn get_browser_ui_test_version(npm: &Path) -> Option<String> { | ||
get_browser_ui_test_version_inner(npm, false) | ||
.or_else(|| get_browser_ui_test_version_inner(npm, true)) | ||
} | ||
|
||
fn compare_browser_ui_test_version(installed_version: &str, src: &Path) { | ||
match fs::read_to_string( | ||
src.join("src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version"), | ||
) { | ||
Ok(v) => { | ||
if v.trim() != installed_version { | ||
eprintln!( | ||
"⚠️ Installed version of browser-ui-test (`{}`) is different than the \ | ||
one used in the CI (`{}`)", | ||
installed_version, v | ||
); | ||
eprintln!( | ||
"You can install this version using `npm update browser-ui-test` or by using \ | ||
`npm install browser-ui-test@{}`", | ||
v, | ||
); | ||
} | ||
} | ||
Err(e) => eprintln!("Couldn't find the CI browser-ui-test version: {:?}", e), | ||
} | ||
fn get_desired_browser_ui_test_version(src: &Path) -> Result<String, io::Error> { | ||
Ok("0.21.1") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, that's being added in a future PR that I want to make this CI change before. once we have a package.json we could update this function to use I suppose probably we'll just use |
||
} | ||
|
||
fn find_librs<P: AsRef<Path>>(path: P) -> Option<PathBuf> { | ||
|
@@ -71,27 +27,6 @@ fn find_librs<P: AsRef<Path>>(path: P) -> Option<PathBuf> { | |
fn main() -> Result<(), ()> { | ||
let config = Arc::new(Config::from_args(env::args().collect())); | ||
|
||
// The goal here is to check if the necessary packages are installed, and if not, we | ||
// panic. | ||
match get_browser_ui_test_version(&config.npm) { | ||
Some(version) => { | ||
// We also check the version currently used in CI and emit a warning if it's not the | ||
// same one. | ||
compare_browser_ui_test_version(&version, &config.rust_src); | ||
} | ||
None => { | ||
eprintln!( | ||
r#" | ||
error: rustdoc-gui test suite cannot be run because npm `browser-ui-test` dependency is missing. | ||
|
||
If you want to install the `browser-ui-test` dependency, run `npm install browser-ui-test` | ||
"#, | ||
); | ||
|
||
panic!("Cannot run rustdoc-gui tests"); | ||
} | ||
} | ||
|
||
let src_path = config.rust_src.join("tests/rustdoc-gui/src"); | ||
for entry in src_path.read_dir().expect("read_dir call failed") { | ||
if let Ok(entry) = entry { | ||
|
@@ -138,16 +73,16 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse | |
} | ||
} | ||
|
||
let mut command = Command::new(&config.nodejs); | ||
let local_node_modules = npm::install_one( | ||
&config.out_dir, | ||
"browser-ui-test", | ||
&get_desired_browser_ui_test_version(&config.rust_src) | ||
.expect("unable to get desired version for browser-ui-test") | ||
.trim(), | ||
) | ||
.expect("unable to install browser-ui-test"); | ||
|
||
if let Ok(current_dir) = env::current_dir() { | ||
let local_node_modules = current_dir.join("node_modules"); | ||
if local_node_modules.exists() { | ||
// Link the local node_modules if exists. | ||
// This is useful when we run rustdoc-gui-test from outside of the source root. | ||
env::set_var("NODE_PATH", local_node_modules); | ||
} | ||
} | ||
let mut command = Command::new(&config.nodejs); | ||
|
||
command | ||
.arg(config.rust_src.join("src/tools/rustdoc-gui/tester.js")) | ||
|
@@ -158,6 +93,12 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse | |
.arg("--tests-folder") | ||
.arg(config.rust_src.join("tests/rustdoc-gui")); | ||
|
||
if local_node_modules.exists() { | ||
// Link the local node_modules if exists. | ||
// This is useful when we run rustdoc-gui-test from outside of the source root. | ||
command.env("NODE_PATH", local_node_modules); | ||
} | ||
|
||
for file in &config.goml_files { | ||
command.arg("--file").arg(file); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use
build.npm
from the bootstrap config instead of being hardcoded.