Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Reproduce #782 as an end-to-end test case #71

Closed

Conversation

adaszko
Copy link
Contributor

@adaszko adaszko commented Nov 22, 2021

Reproduces (with a failure because it's still unresolved) the issue radicle-dev/radicle-link#782

Open questions/decisions:

  • Whether we want to keep the monorepo contents that trigger the issue within this repository, as it adds (acceptably IMO) to to this repo size. Then again, it's the most convenient option compared to fetching it from external sources.
  • It needs to be added to CI on this repo, but since the test is going to fail anyway, I held that back for now.

How it works: The Python code launches 2 separate org-node processes and sets up the 2nd one (the replicator) to replicate from the 1st one (the bootstrap). Whenever it sees a log entry of severity ERROR on either of the stdouts of the two, it fails the test. After it infers (based on logs) that the replication completed, it does asserts on the existence of relevant files that should be present after replication.

The README file has instructions on how to run the test.

This style of writing tests should be flexible enough to reproduce many of the issues rounded up in radicle-dev/radicle-upstream#2449 (comment)

@adaszko adaszko requested a review from cloudhead November 22, 2021 14:59
@adaszko adaszko force-pushed the adaszko/reproduce-issue-782 branch from 34c3e4d to 6b705b9 Compare November 22, 2021 15:09
@adaszko adaszko changed the title GitHub issue #782 reproduction as a end-to-end test case GitHub issue #782 reproduction as an end-to-end test case Nov 22, 2021
@adaszko adaszko changed the title GitHub issue #782 reproduction as an end-to-end test case Reproduce #782 as an end-to-end test case Nov 22, 2021
@adaszko adaszko force-pushed the adaszko/reproduce-issue-782 branch from 6b705b9 to f27d630 Compare November 22, 2021 15:11
@cloudhead
Copy link
Contributor

This is really great! I wonder though if it belongs here, vs. a dedicated repo? Since the changes that would break or fix this test will usually be in the underlying protocols.

@adaszko
Copy link
Contributor Author

adaszko commented Nov 24, 2021

This is really great! I wonder though if it belongs here, vs. a dedicated repo? Since the changes that would break or fix this test will usually be in the underlying protocols.

Thanks! I think the classic rule of "late bugs cost more to fix" applies here. Ideally, we would want to execute end-to-end tests on each PR, even before merging to master to see if a PR introduces a regression. The PR may be a change exclusive to the client services code, or it may be a version bump of librad and friends that it depends on. In both cases, it's valuable to see right away if something broke. If those tests land in a separate repository, that's going to be harder to pull off.

Another point is that those tests aren't limited to testing the protocol. They may as well check if org-node produces proper outputs on the newly introduced built-in http server.

@adaszko adaszko force-pushed the adaszko/reproduce-issue-782 branch 5 times, most recently from 398e99a to 70bdba7 Compare November 26, 2021 15:58
@adaszko adaszko force-pushed the adaszko/reproduce-issue-782 branch from 70bdba7 to f37b3a3 Compare November 26, 2021 16:01
@adaszko
Copy link
Contributor Author

adaszko commented Nov 26, 2021

I've managed to minimize the input monorepo to just two repos: hnrkjajuucc6zp5eknt3s9xykqsrus44cjimy andhnrkbtw9t1of4ykjy6er4qqwxtc54k9943eto. If I remove the former, the test case passes. The latter can't be removed because it wouldn't be replicated and it is at the replication of it that the error occurs, so I think the test case is small as it gets now.

As an aside: tarring and even compressing didn't make a substantial difference, so I've forgone it because it makes working with code harder at no noticeable gain.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants