`CONTRIBUTING.md` and Testing changes #48

award28 · 2023-05-03T01:22:18Z

Per @mpalmer's request in #39, I've extracted the testing and contributing changes into a separate PR. This still includes the changes to save-snapshot.

@mpalmer
Should we still maintain the run test binary? I don't see a reason why we would need to, but there may be something I'm not thinking of.

@bcheidemann From your comment here you mention

I think some minor updates are needed to tests/run.mjs (updating test to tests) and also the validation_state.snap.json files need to be added for the new test cases.

Could you lend me a hand on this PR to get those json files to work with the new testing approach?

…on-validator into aw-contributing-and-testing

bcheidemann · 2023-05-03T11:22:03Z

Could you lend me a hand on this PR to get those json files to work with the new testing approach?

@award28 Yeah of course :) There's two approaches we could take:

"fix" run.mjs now (this would be a very minor change) and integrate the testing approach in a follow-up PR
Integrate the testing approach in this PR and remove run.mjs

I've raised a quick fix for run.mjs in this PR. I'll start working on the changes needed to remove run.mjs now :) my preference would be to merge these changes as a separate PR since I don't want to risk unnecessarily delaying this one

award28 · 2023-05-03T11:48:23Z

Definitely agree, thanks for the quick turn around! I've merged those changes into this branch. Based on feedback from #39, I should be good to remove the run binary and update CI.

award28 · 2023-05-09T17:04:43Z

@mpalmer bumping this in case you missed it

mpalmer · 2023-05-10T03:12:25Z

Thanks for the bump, @award28, this slipped off my radar. I've put this on my list to review tomorrow; to answer your question:

Should we still maintain the run test binary?

Burninate it.

mpalmer

Very, very cool. Made a couple of copy edit suggestions, and I've got a question or two, but it's great stuff, definite improvement over my shell script.

mpalmer · 2023-05-10T04:58:24Z

.github/workflows/qa.yml

-        uses: mfinelli/setup-shfmt@master
-
-      - name: Run shfmt
-        run: shfmt -d bin/* test/run


While test/run is going away, the scripts in bin/ are going to hang around.

mpalmer · 2023-05-10T04:59:53Z

CONTRIBUTING.md

+# Environment Setup
+
+## Install Rust
+Firstly, you'll need make any changes to the core functionality of this project. We recommend use `rustup`, on the recommendation of the rust team. You can find the installation instructions [here](https://www.rust-lang.org/tools/install).


Bit of light copy-editing:

Suggested change

Firstly, you'll need make any changes to the core functionality of this project. We recommend use `rustup`, on the recommendation of the rust team. You can find the installation instructions [here](https://www.rust-lang.org/tools/install).

Firstly, you'll need a Rust toolchain to make any changes to the core functionality of this project. We recommend [using `rustup`](https://www.rust-lang.org/tools/install), because that's what the Rust core team recommend.

mpalmer · 2023-05-10T08:54:29Z

CONTRIBUTING.md

+Submodule path 'src/schemastore': checked out 'd3e6ab7727380b214acbab05570fb09a3e5d2dfc'
+```
+
+At this point, you should be all set to `cargo run`! If you run into any issues here, please [create an issue](https://github.com/mpalmer/action-validator/issues/new/choose).


Suggested change

At this point, you should be all set to `cargo run`! If you run into any issues here, please [create an issue](https://github.com/mpalmer/action-validator/issues/new/choose).

At this point, you should be all set to `cargo run`!

Given we've got CI and (as I'm about to commit) nightly test builds, I'm willing to bet a reasonable sum that if cargo run doesn't work for someone, it's a problem on their end, and I, at least, don't have the capacity to provide tech support for peoples' Rust installs.

mpalmer · 2023-05-10T08:55:38Z

CONTRIBUTING.md

+When the tests is run, the results of the test must exactly match those of the previous run. For this project,
+the snapshot tests are named in the format `{next_id}_{whats_being_tested}` (e.g. `011_remote_checks_failure`).
+
+If you have made changes which will change the output of the program and cause snapshots to fail, you can run


Ooooh, shiny!

mpalmer · 2023-05-10T08:56:44Z

CONTRIBUTING.md

@@ -1,3 +1,5 @@
+# Overview


This is all good stuff, thanks for writing it up.

mpalmer · 2023-05-10T09:09:34Z

tests/snapshot_tests.rs

+
+
+#[rstest]
+#[case("001_basic_workflow")]


Any chance we can sneak the list of tests to run straight off the filesystem, rather than having to list them here? Alternately, it seems like we're not exactly stretching the limits of rstest; perhaps it's overkill, and just a fn main() and a loop would do the trick for now?

That seems like a very neat way of going about it. Macro magic to the rescue!

mpalmer · 2023-05-10T09:10:21Z

tests/snapshot_tests.rs

+        }
+    }
+
+    fn _save_contents(


What's the rationale for the _ prefix on these fns?

bcheidemann · 2023-09-17T22:05:48Z

Any update on this? Happy to pick up the changes requested if you don't have capacity @award28 :)

mpalmer · 2024-01-30T00:44:14Z

Hey @award28, do you have the bandwidth to push this through to completion, or shall @bcheidemann and myself finish it off and land it?

award28 added 2 commits March 7, 2023 08:57

Add sections to contributing guide

a11e734

Add testing and contrib changes

2928a74

award28 force-pushed the aw-contributing-and-testing branch from 53ffc60 to 2928a74 Compare May 3, 2023 01:31

award28 added 3 commits May 2, 2023 21:31

Merge branch 'main' into aw-contributing-and-testing

ee55ae7

Remove remote checks tests

a04ef56

Merge branch 'aw-contributing-and-testing' of github.com:award28/acti…

adfe8f0

…on-validator into aw-contributing-and-testing

award28 marked this pull request as ready for review May 3, 2023 01:36

award28 mentioned this pull request May 3, 2023

38 validate action version exists #39

Open

1 task

fix: run.mjs parent directory renaming

f86098c

Merge pull request #1 from bcheidemann/fix/run_mjs

d647f25

Update qa job and js package tests; remove run binary

a7a20f1

mpalmer reviewed May 10, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`CONTRIBUTING.md` and Testing changes #48

`CONTRIBUTING.md` and Testing changes #48

award28 commented May 3, 2023 •

edited

Loading

bcheidemann commented May 3, 2023

award28 commented May 3, 2023

award28 commented May 9, 2023

mpalmer commented May 10, 2023

mpalmer left a comment

mpalmer May 10, 2023

mpalmer May 10, 2023

mpalmer May 10, 2023

mpalmer May 10, 2023

mpalmer May 10, 2023

mpalmer May 10, 2023

bcheidemann Sep 17, 2023

mpalmer Sep 18, 2023

mpalmer May 10, 2023

bcheidemann commented Sep 17, 2023

mpalmer commented Jan 30, 2024

	Firstly, you'll need make any changes to the core functionality of this project. We recommend use `rustup`, on the recommendation of the rust team. You can find the installation instructions [here](https://www.rust-lang.org/tools/install).
	Firstly, you'll need a Rust toolchain to make any changes to the core functionality of this project. We recommend [using `rustup`](https://www.rust-lang.org/tools/install), because that's what the Rust core team recommend.

	At this point, you should be all set to `cargo run`! If you run into any issues here, please [create an issue](https://github.com/mpalmer/action-validator/issues/new/choose).
	At this point, you should be all set to `cargo run`!



		#[rstest]
		#[case("001_basic_workflow")]

CONTRIBUTING.md and Testing changes #48

Are you sure you want to change the base?

CONTRIBUTING.md and Testing changes #48

Conversation

award28 commented May 3, 2023 • edited Loading

bcheidemann commented May 3, 2023

award28 commented May 3, 2023

award28 commented May 9, 2023

mpalmer commented May 10, 2023

mpalmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcheidemann commented Sep 17, 2023

mpalmer commented Jan 30, 2024

`CONTRIBUTING.md` and Testing changes #48

`CONTRIBUTING.md` and Testing changes #48

award28 commented May 3, 2023 •

edited

Loading