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

Disable documents test on processors metanorma-* #130

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

CAMOBAP
Copy link
Contributor

@CAMOBAP CAMOBAP commented Nov 15, 2023

This PR to discuss our metanorma test approach

Recently @opoudjis asked me:

are the samples tests in metanorma-* now redundant?
... Just want to make sure documents aren't being tested twice

For now we do testing documents in 3 places:

  1. on metanorma-* gem changes, we test only specific mn-samples-* and mn-templates-*
  2. on metanorma-cli gem changes, we test subset of all our mn-samples-* and mn-templates-*, king of smoke test
  3. on mn-samples-* and mn-templates-*changes

@opoudjis proposal is to remove processor tests (first row from the list above)

Pros:

  • Reduce the ci time
  • Simplify ci configuration

Cons:

  • We will identify the issue much later in time i.e. on metanorma-cli run

@ronaldtse any thoughts?

P.S. If we can make development more comfortable for @opoudjis I have no objections to go with this approach

@CAMOBAP CAMOBAP self-assigned this Nov 15, 2023
@opoudjis
Copy link
Contributor

The way I do development, the long wait on flavour testing is more disruptive than an issue that turns up in documents or templates and not identified in the flavour rspec. The former happens constantly, the latter only happens every few months. I don't think that there is a real problem in identifying flavour-specific issues only when I get to metanorma-cli: the release takes long enough that stopping to debug the flavour (after a very very long wait) is going to end up happening the next day anyway.

@CAMOBAP
Copy link
Contributor Author

CAMOBAP commented Nov 18, 2023

@ronaldtse if no objections, I propose to merge this PR

@ronaldtse
Copy link
Contributor

@CAMOBAP @opoudjis I can live with the new proposal. Feel free to merge when ready. Thanks!

@CAMOBAP CAMOBAP merged commit 8d3882c into main Nov 21, 2023
53 of 54 checks passed
@CAMOBAP CAMOBAP deleted the feature/disable-document-testing-on-processor-gems branch November 21, 2023 07:21
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.

3 participants