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

Add: Javascript integration tests with yarn link #114

Merged
merged 17 commits into from
Dec 21, 2022

Conversation

dynamite-bud
Copy link
Contributor

Javascript Testing in Integration Suite

Using:

  • yarn link
  • reading created package.json using serde_json

Not Done:

  • Error handling not done for serde

@github-actions
Copy link

github-actions bot commented Dec 15, 2022

⏱ Workflow Timer ⏱

Make sure you keep an eye on build times!

One of this project's goals is to keep CI runs under 5 minutes so developers can maintain fast edit-compile-test cycles.

Run Compile and Test
master 12mins 3s
ca417c8 6mins 14s

🤖 Beep. Boop. I'm a bot. If you find any issues, please report them to https://github.com/Michael-F-Bryan/workflow-timer.

@dynamite-bud dynamite-bud marked this pull request as draft December 19, 2022 14:12
@dynamite-bud dynamite-bud marked this pull request as ready for review December 19, 2022 14:12
Comment on lines +57 to +64
// println!("{:?}", manifest_path);

Copy link
Contributor

Choose a reason for hiding this comment

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

This can be deleted.

@@ -1,18 +0,0 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to delete this lockfile?

@@ -15,6 +15,7 @@ fn initialize_logging() {
}

#[test]
#[ignore = "Python tests not working on mac os"]
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to refine this so it only ignores the test on macos.

Suggested change
#[ignore = "Python tests not working on mac os"]
#[cfg_attr(target_os = "macos", ignore = "Python tests not working on mac os")]

The #[cfg_attr] attribute is useful for enabling attributes only under certain conditions.

Comment on lines 54 to 55
// setup_python(crate_dir, &bindings)?;
// run_pytest(crate_dir)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of commenting these lines out, let's update run_pytest() so it does a check for MacOS at the start.

fn run_pytest(crate_dir: &Path) -> Result<(), Error> {
  if cfg!(target_os = "macos") {
    tracing::warn!(
      issue = "https://github.com/wasmerio/wasmer-python/issues/657", 
      "Skipping python tests on MacOS",
    );
    return Ok(());
  }

  ...
}

@@ -127,7 +136,7 @@ fn language_specific_matches(package_dir: &Path, language: Language) -> Result<W
let mut builder = OverrideBuilder::new(package_dir);

let overrides = match language {
Language::JavaScript => todo!(),
Language::JavaScript => builder.add("*.js")?.add("*.test.js")?.build()?,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we also want to add *.ts and *.mjs?

crates/testing/src/autodiscover.rs Outdated Show resolved Hide resolved
Comment on lines +247 to +249
let mut cmd = Command::new("yarn");
cmd.arg("init").arg("--yes").current_dir(crate_dir);
tracing::info!(?cmd, "Initializing the Javascript package");
Copy link
Contributor

Choose a reason for hiding this comment

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

We're always going to initialize a new package.json even if one already exists, won't we?

crates/testing/src/autodiscover.rs Outdated Show resolved Hide resolved
crates/testing/src/autodiscover.rs Outdated Show resolved Hide resolved
crates/testing/src/autodiscover.rs Outdated Show resolved Hide resolved
@dynamite-bud dynamite-bud merged commit b777996 into master Dec 21, 2022
@dynamite-bud dynamite-bud deleted the javascript-testing branch December 21, 2022 11:04
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.

2 participants