Skip to content

File glibtest#761

Merged
sporksmith merged 2 commits intoshadow:masterfrom
sporksmith:file-glibtest
Apr 14, 2020
Merged

File glibtest#761
sporksmith merged 2 commits intoshadow:masterfrom
sporksmith:file-glibtest

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

Enables individual file tests to be turned on and off independently, so that we can enable them incrementally in the phantom branch.

Migrates to glib's test framework so that we can filter which tests run from the command-line. This also makes the test code more succinct and give better failure reporting.

Modifies subtests to be independent of each-other; i.e. not depend on earlier tests to have created or modified a test file.

This will let us more cleanly enable/disable individual sub-tests in the
Phantom branch.
This will allow us to turn tests on/off without affecting each-other.
In general making individual tests independent of each-other makes them
easier to understand and debug.
@sporksmith sporksmith requested a review from robgjansen April 14, 2020 01:33
Copy link
Copy Markdown
Member

@robgjansen robgjansen left a comment

Choose a reason for hiding this comment

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

My main concern is that we'll be rewriting some tests twice, once for glibtest and again for rust. But I suppose that's OK if we don't want to pull in Rust yet at this point.

@sporksmith
Copy link
Copy Markdown
Contributor Author

My main concern is that we'll be rewriting some tests twice, once for glibtest and again for rust. But I suppose that's OK if we don't want to pull in Rust yet at this point.

Yeah, tweaking the tests to use the glib framework is probably faster than rewriting them in Rust; at least faster than adding the first Rust code/test, but I agree overall it's adding an extra step if we eventually want to migrate to Rust.

Maybe with this test merged so that you have something to work with, I'll take a look at using Rust for the udp test.

@sporksmith sporksmith merged commit 74d0c9e into shadow:master Apr 14, 2020
@sporksmith sporksmith deleted the file-glibtest branch April 14, 2020 14:56
@robgjansen
Copy link
Copy Markdown
Member

robgjansen commented Apr 14, 2020

Maybe with this test merged so that you have something to work with, I'll take a look at using Rust for the udp test.

I guess this introduces a larger question about plans for our next release. Do we intend to release phantom changes before we start migrating the wider Shadow code-base to Rust? Or should we keep the phantom changes in a [public] dev branch as we continue improving Shadow in other ways, but without making a formal release? If we do the former, we may not want to introduce Rust as yet another dependency yet. But if we do the latter, which I'm in favor of, I would say we could go ahead with Rust whenever we feel ready.

@sporksmith
Copy link
Copy Markdown
Contributor Author

Suppose we create a shadow/shadow:v1 branch from the current shadow/shadow:master, and then go ahead with adding Rust to master? That lets us keep non-Phantom-specific development public and leaves us a stable branch where we can backport fixes/improvements if needed. It also leaves our options open as to whether to backport the Rust migration into the v1 series or not. (I'm assuming an eventual Phantom release would be v2.0.0)

@robgjansen
Copy link
Copy Markdown
Member

Seems reasonable, but let's discuss at our meeting next week before taking action. That way we can include Ryan in the discussion.

@sporksmith sporksmith mentioned this pull request Apr 15, 2020
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