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

port selection and test config generation macro #528

Merged
merged 9 commits into from
Jan 24, 2024

Conversation

zeeshanlakhani
Copy link
Contributor

Description

@euamcg added a macro for inlining toml configs for tests. As we can see the amount of .toml configs is getting quite large. The macro allows us to write toml configs inside tests, create a fixture file, and discard it when the test is finished.

This closes #485 and #491 (for now, as we may come back to some optimizations down the line related to the latter).

Link to issue

Type of change

  • Refactor (non-breaking change that updates existing functionality)
  • This change requires a documentation update

Please delete options that are not relevant.

Test plan (required)

All tests are passing.

@zeeshanlakhani zeeshanlakhani requested a review from a team as a code owner January 23, 2024 19:53
@zeeshanlakhani zeeshanlakhani changed the title Euanmcg/zl/toml generation macro port selection and test config generation macro Jan 23, 2024
@zeeshanlakhani zeeshanlakhani mentioned this pull request Jan 23, 2024
2 tasks
Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (f29d4d4) 67.52% compared to head (80bfdbc) 67.46%.
Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #528      +/-   ##
==========================================
- Coverage   67.52%   67.46%   -0.07%     
==========================================
  Files          83       83              
  Lines        9635     9635              
==========================================
- Hits         6506     6500       -6     
- Misses       3129     3135       +6     
Files Coverage Δ
homestar-runtime/src/network/webserver.rs 96.30% <100.00%> (ø)
homestar-runtime/src/runner.rs 61.89% <100.00%> (ø)
homestar-runtime/src/settings.rs 98.24% <100.00%> (ø)
homestar-runtime/src/metrics/exporter.rs 83.78% <0.00%> (ø)

... and 4 files with indirect coverage changes

@zeeshanlakhani zeeshanlakhani force-pushed the euanmcg/zl/toml_generation_macro branch 2 times, most recently from 6a4bae8 to dc9e3b6 Compare January 23, 2024 21:03
Co-authored-by: Euan McGinness <[email protected]>
@zeeshanlakhani zeeshanlakhani force-pushed the euanmcg/zl/toml_generation_macro branch from dc9e3b6 to 0778086 Compare January 23, 2024 21:07
@zeeshanlakhani zeeshanlakhani deleted the euanmcg/zl/toml_generation_macro branch January 23, 2024 22:11
@zeeshanlakhani zeeshanlakhani restored the euanmcg/zl/toml_generation_macro branch January 23, 2024 22:14
homestar-runtime/src/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@bgins bgins left a comment

Choose a reason for hiding this comment

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

Looks great! Nice to get rid of all those fixtures, loving the inline TOML. ❤️

Comment on lines +273 to +293
impl ProcInfo {
pub(crate) fn new() -> Result<Self> {
let uuid = &uuid::Uuid::new_v4();
let name = format!("tests/fixtures/{}.db", uuid);

let proc_info = Self {
metrics_port: port_selector::random_free_tcp_port()
.ok_or(anyhow::anyhow!("failed to select free port for metrics"))?,
rpc_port: RpcSelector::new().select_free_port()?,
ws_port: port_selector::random_free_port().ok_or(anyhow::anyhow!(
"failed to select free port for JSON-RPC webserver"
))?,
db_path: PathBuf::from_str(&name)?,
listen_port: port_selector::random_free_port().ok_or(anyhow::anyhow!(
"failed to select free port for listen address"
))?,
};

Ok(proc_info)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Good stuff 💯

Comment on lines +391 to +409
macro_rules! make_config {
// For tests where all you want to do is write toml.
($toml:expr) => {{
let uuid = uuid::Uuid::new_v4();
let name = format!("tests/fixtures/{}.toml", uuid);
let toml = $toml.parse::<toml::Table>().unwrap();
let test_config = $crate::utils::TestConfig::new(name.to_string(), toml);
test_config.save_fixture().unwrap();
test_config
}};
// For some finer control over the config.
($name:expr, $toml:expr) => {{
let name = format!("tests/fixtures/{}.toml", $name);
let toml = $toml.parse::<toml::Table>().unwrap();
let test_config = $crate::utils::TestConfig::new(name.to_string(), toml);
test_config.save_fixture().unwrap();
test_config
}};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

💖

@zeeshanlakhani zeeshanlakhani merged commit e25385c into main Jan 24, 2024
31 checks passed
@zeeshanlakhani zeeshanlakhani deleted the euanmcg/zl/toml_generation_macro branch January 24, 2024 01:24
@release-plz-ipvm-wg release-plz-ipvm-wg bot mentioned this pull request Jan 23, 2024
zeeshanlakhani pushed a commit that referenced this pull request Feb 21, 2024
## 🤖 New release
* `homestar-runtime`: 0.1.1 -> 0.2.0 (⚠️ API breaking changes)
* `homestar-invocation`: 0.1.1 -> 0.2.0 (✓ API compatible changes)
* `homestar-wasm`: 0.1.1 -> 0.2.0 (✓ API compatible changes)
* `homestar-workflow`: 0.1.1 -> 0.2.0 (✓ API compatible changes)

### ⚠️ `homestar-runtime` breaking changes

```
--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.28.0/src/lints/enum_variant_added.ron

Failed in:
  variant Command:Node in /tmp/.tmp7PLiiL/homestar/homestar-runtime/src/cli.rs:149
  variant Command:Info in /tmp/.tmp7PLiiL/homestar/homestar-runtime/src/cli.rs:155
```

<details><summary><i><b>Changelog</b></i></summary><p>

## `homestar-runtime`
<blockquote>

##
[0.2.0](homestar-runtime-v0.1.1...homestar-runtime-v0.2.0)
- 2024-02-20

### Added
- Add OpenRPC API docs and associated JSON Schemas
([#534](#534))
- redial `node_addresses` at an interval on connection close
([#529](#529))

### Fixed
- add handling of dns multiaddrs + bootstrapping + CLI / Conn changes
([#547](#547))

### Other
- deps + flake cleanup
([#581](#581))
- Allow dead code default timeout
([#577](#577))
- Update homestar-functions to use cargo component
([#576](#576))
- fix transport order for wss possibility
([#563](#563))
- small comment, sorry
([#561](#561))
- move away from deadlines dealing w/ the runner and wasi-preview 2
wasmtime ([#560](#560))
- docker updates with info command and rpc host update
([#558](#558))
- just test conn ([#544](#544))
- handle this evil workflow_info test
([#543](#543))
- remove unnecessary deps and add tooling for those checks
([#541](#541))
- [chore(cargo)](deps): bump puffin from 0.18.1 to 0.19.0
([#537](#537))
- updates/flaky kills on ci
([#540](#540))
- release docs and cp readmes
([#530](#530))
- port selection and test config generation macro
([#528](#528))
- [chore(cargo)](deps): bump serde_with from 3.4.0 to 3.5.0
([#524](#524))
- [chore(cargo)](deps): bump moka from 0.12.3 to 0.12.4
([#525](#525))
</blockquote>

## `homestar-invocation`
<blockquote>

##
[0.2.0](homestar-invocation-v0.1.1...homestar-invocation-v0.2.0)
- 2024-02-20

### Added
- Add OpenRPC API docs and associated JSON Schemas
([#534](#534))

### Other
- deps + flake cleanup
([#581](#581))
- Update homestar-functions to use cargo component
([#576](#576))
- move away from deadlines dealing w/ the runner and wasi-preview 2
wasmtime ([#560](#560))
- remove unnecessary deps and add tooling for those checks
([#541](#541))
- release docs and cp readmes
([#530](#530))
</blockquote>

## `homestar-wasm`
<blockquote>

##
[0.2.0](homestar-wasm-v0.1.1...homestar-wasm-v0.2.0)
- 2024-02-20

### Other
- deps + flake cleanup
([#581](#581))
- Update homestar-functions to use cargo component
([#576](#576))
- move away from deadlines dealing w/ the runner and wasi-preview 2
wasmtime ([#560](#560))
- remove unnecessary deps and add tooling for those checks
([#541](#541))
- release docs and cp readmes
([#530](#530))
</blockquote>

## `homestar-workflow`
<blockquote>

##
[0.2.0](homestar-workflow-v0.1.1...homestar-workflow-v0.2.0)
- 2024-02-20

### Added
- Add OpenRPC API docs and associated JSON Schemas
([#534](#534))

### Other
- deps + flake cleanup
([#581](#581))
- remove unnecessary deps and add tooling for those checks
([#541](#541))
- release docs and cp readmes
([#530](#530))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Signed-off-by: release-plz-ipvm-wg[bot] <144082651+release-plz-ipvm-wg[bot]@users.noreply.github.com>
Co-authored-by: release-plz-ipvm-wg[bot] <144082651+release-plz-ipvm-wg[bot]@users.noreply.github.com>
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.

Testing: Integration tests should use inlined configuration vs fixture/test-config files
3 participants