/qubes-issues

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

Run automated tests on PRs before merging #2725

Closed
andrewdavidwong opened this Issue Mar 24, 2017 · 11 comments

Comments

Assignees

@marmarek marmarek

Labels
Projects
None yet
Milestone
  Ongoing
3 participants
@andrewdavidwong @marmarek @jpouellet
@andrewdavidwong
Member

andrewdavidwong commented Mar 24, 2017

@marmarek: Is there a way we can run automated Travis CI test on qubes-doc and website PRs before merging them? I often find that I merge a qubes-doc PR only to have it report a broken link after your automated script updates the submodule. It would be great for PR contributors to be able to see that their PR is going to break a link and fix it before I merge.

@andrewdavidwong andrewdavidwong added C: website enhancement labels Mar 24, 2017

@andrewdavidwong andrewdavidwong added this to the Documentation/website milestone Mar 24, 2017

@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Mar 24, 2017

Member

Will look into it, shouldn't be that hard.
Member

marmarek commented Mar 24, 2017

Will look into it, shouldn't be that hard.

@jpouellet jpouellet referenced this issue Apr 17, 2017

Closed

site: broken links on download page #2752

@jpouellet

This comment has been minimized.

Show comment
Hide comment
@jpouellet

jpouellet Apr 19, 2017

Contributor

I have some WIP initial testing in here and here. General idea is to always checkout qubesos.github.io, have a _utils/travis.sh script there which properly checks out upstream version of all submodules, substituting specific repo under test. Then run tests on that.

WIP version is broken because it uses master rather than the PR. Fix should be simple but I won't have time to work on this (or really anything qubes-related) for the next few weeks, so just sharing WIP version in case anyone wants to pick it up in the mean time.

Once that works, I've also implemented incremental spell checking. Will merge after.
Contributor

jpouellet commented Apr 19, 2017

I have some WIP initial testing in here and here. General idea is to always checkout qubesos.github.io, have a _utils/travis.sh script there which properly checks out upstream version of all submodules, substituting specific repo under test. Then run tests on that.

WIP version is broken because it uses master rather than the PR. Fix should be simple but I won't have time to work on this (or really anything qubes-related) for the next few weeks, so just sharing WIP version in case anyone wants to pick it up in the mean time.

Once that works, I've also implemented incremental spell checking. Will merge after.
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Apr 19, 2017

Member

The easiest way to make it working on the right sources (PR instead of master) is rely on Travis and use whatever it downloaded. Like here:
https://github.com/QubesOS/qubes-builder/blob/master/scripts/travis-build#L63-L67
Member

marmarek commented Apr 19, 2017

The easiest way to make it working on the right sources (PR instead of master) is rely on Travis and use whatever it downloaded. Like here:
https://github.com/QubesOS/qubes-builder/blob/master/scripts/travis-build#L63-L67
@jpouellet

This comment has been minimized.

Show comment
Hide comment
@jpouellet

jpouellet May 6, 2017

Contributor

@marmarek I see you've started merging this. A few notes:

  1. The reason I had a separate old_site and new_site dirs checked out in separate locations simultaneously is because I found some checks (for example reporting words introduced by the PR not found in the dictionary nor in existing documentation text) seem to make the most sense to implement by building both new and old _sites and diffing results of scripts run each.
  2. I intentionally used all repos of the owner of the "base" of the PR rather than hard-coding QubesOS, which means it's possible to test net effect of changes in multiple repos before sending any PR to QubesOS/*. IMO that behavior is worth retaining.

I'm in final exams right now, otherwise I'd have sent PRs already :)
Contributor

jpouellet commented May 6, 2017

@marmarek I see you've started merging this. A few notes:

  1. The reason I had a separate old_site and new_site dirs checked out in separate locations simultaneously is because I found some checks (for example reporting words introduced by the PR not found in the dictionary nor in existing documentation text) seem to make the most sense to implement by building both new and old _sites and diffing results of scripts run each.
  2. I intentionally used all repos of the owner of the "base" of the PR rather than hard-coding QubesOS, which means it's possible to test net effect of changes in multiple repos before sending any PR to QubesOS/*. IMO that behavior is worth retaining.

I'm in final exams right now, otherwise I'd have sent PRs already :)
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek May 6, 2017

Member
Member

marmarek commented May 6, 2017

marmarek added a commit to QubesOS/qubesos.github.io that referenced this issue May 6, 2017

@marmarek
travis: refactor .travis.yml to a script for reuse in submodules 
QubesOS/qubes-issues#2725

Squashed commit of the following:

commit 7481878
Author: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Date:   Sat May 6 03:32:51 2017 +0200

    travis: use trusty image

commit 098c16a
Author: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Date:   Sat May 6 01:56:26 2017 +0200

    travis: run test on PR, not base version

    Also, always use what travis have fetched, instead of what submodule
    point at. This allow testing pushes to submodules before updating main
    repository (alternative branches etc).

commit df6d294
Author: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Date:   Sat May 6 01:55:08 2017 +0200

    travis: use Travis-downloaded sources for test

commit 9c2a270
Merge: 480ece1 0919f67
Author: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Date:   Sat May 6 01:43:46 2017 +0200

    Merge remote-tracking branch 'jpo-bot/travis' into travis

commit 0919f67
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 16:19:16 2017 -0400

    Fix / escaping in repo url

commit b0a3822
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 16:08:43 2017 -0400

    Travis actually works for qubes-doc now

commit 6e60d8f
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 03:57:19 2017 -0400

    More travis fixes

commit 6547e55
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 03:50:30 2017 -0400

    Travis fixes

commit bc3dda9
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 03:39:38 2017 -0400

    More travis improvements

commit b410333
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 02:40:44 2017 -0400

    Remove -z :(

commit 6a9c035
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 02:33:29 2017 -0400

    Only match repo name, not owner

commit b6ddfdf
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 02:27:07 2017 -0400

    Remove unportable -z

commit 1193179
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 02:15:03 2017 -0400

    Various travis improvements

commit fe22ca0
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Tue Apr 18 02:14:37 2017 -0400

    Trace everything in travis script

commit 243fbe2
Author: Jean-Philippe Ouellet <jpo@vt.edu>
Date:   Mon Apr 17 23:47:09 2017 -0400

    Move travis tasks into script

    so we can clone & call it from travis hooks in other repos.
Verified
This commit was signed with a verified signature.
@marmarek marmarek Marek Marczykowski-Górecki
GPG key ID: F32894BE9684938A Learn about signing commits
Loading status checks…
69d01c1

@marmarek marmarek closed this in QubesOS/qubes-doc@3ae2b4f May 6, 2017

marmarek added a commit to QubesOS/qubes-posts that referenced this issue May 6, 2017

@marmarek
travis: add .travis.yml 
Fixes QubesOS/qubes-issues#2725
Verified
This commit was signed with a verified signature.
@marmarek marmarek Marek Marczykowski-Górecki
GPG key ID: F32894BE9684938A Learn about signing commits
b8c138b
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek May 6, 2017

Member

Merged work by @jpouellet . For now I've enabled it for qubes-doc and qubes-posts repos. Unfortunately it will work only for new PRs (made from just updated master) - there is no no easy way to add this to all those existing PRs...
But there is a trick: you can merge a bunch of PRs into some test branch, push it (without signed tag yet) and wait for Travis to check it. If ok, sign and push to master.
Member

marmarek commented May 6, 2017

Merged work by @jpouellet . For now I've enabled it for qubes-doc and qubes-posts repos. Unfortunately it will work only for new PRs (made from just updated master) - there is no no easy way to add this to all those existing PRs...
But there is a trick: you can merge a bunch of PRs into some test branch, push it (without signed tag yet) and wait for Travis to check it. If ok, sign and push to master.
@andrewdavidwong

This comment has been minimized.

Show comment
Hide comment
@andrewdavidwong

andrewdavidwong May 6, 2017

Member

Thanks, @jpouellet and @marmarek!
Member

andrewdavidwong commented May 6, 2017

Thanks, @jpouellet and @marmarek!

@jpouellet jpouellet referenced this issue in QubesOS/qubes-doc May 25, 2017

Merged

Various minor spelling and grammar fixes #421

jpouellet added a commit to jpouellet/qubesos.github.io that referenced this issue Jul 4, 2017

@jpouellet
Differential spell checking 
This script does not attempt to find all misspellings, but rather show
when new words not previously known are introduced. This mean it's
useful in a continuous integration pipeline where we don't necessarily
want each contributor to have to be responsible for the bad spelling of
whoever came before them, but rather at least not make things worse.
The existing spelling can then be incrementally cleaned up.

This approach also means we don't need to maintain a local dictionary to
avoid false-positives, because existing "misspelled words" ("AppVM",
"marmarek", etc.) will be ignored.

In adition, effort is made to only check prose, not code. All the actual
parsing logic is handled by pandoc, and we just filter and transform a
JSON AST that it produces.

We also operate on the fully built HTML site rather than the source
markdown, meaning we see all spellings as they exist post-templating,
including all values from _data/*.yaml and such already filled in.

This was written primarily for use in a CI hook, but is of course also
useful locally. The default invocation checks the net difference between
origin/master and your local HEAD. You can also set SPELL_CHECK_CACHE to
a directory in which to keep results of previous runs, which speeds up
re-checking incremental changes.

Fixes: QubesOS/qubes-issues#2725
Verified
This commit was signed with a verified signature.
@jpouellet jpouellet Jean-Philippe Ouellet
GPG key ID: E0C9723C50A46274 Learn about signing commits
5a0dcf4

jpouellet added a commit to jpouellet/qubesos.github.io that referenced this issue Jul 4, 2017

@jpouellet
Differential spell checking 
This script does not attempt to find all misspellings, but rather show
when new words not previously known are introduced. This mean it's
useful in a continuous integration pipeline where we don't necessarily
want each contributor to have to be responsible for the bad spelling of
whoever came before them, but rather at least not make things worse.
The existing spelling can then be incrementally cleaned up.

This approach also means we don't need to maintain a local dictionary to
avoid false-positives, because existing "misspelled words" ("AppVM",
"marmarek", etc.) will be ignored.

In adition, effort is made to only check prose, not code. All the actual
parsing logic is handled by pandoc, and we just filter and transform a
JSON AST that it produces.

We also operate on the fully built HTML site rather than the source
markdown, meaning we see all spellings as they exist post-templating,
including all values from _data/*.yaml and such already filled in.

This was written primarily for use in a CI hook, but is of course also
useful locally. The default invocation checks the net difference between
origin/master and your local HEAD. You can also set SPELL_CHECK_CACHE to
a directory in which to keep results of previous runs, which speeds up
re-checking incremental changes.

Introduced dependencies are: hunspell (preferred) or aspell, and jq.

Fixes: QubesOS/qubes-issues#2725
Verified
This commit was signed with a verified signature.
@jpouellet jpouellet Jean-Philippe Ouellet
GPG key ID: E0C9723C50A46274 Learn about signing commits
8750464

@jpouellet jpouellet referenced this issue in QubesOS/qubesos.github.io Jul 4, 2017

Closed

Differential spell checking #110

@andrewdavidwong

This comment has been minimized.

Show comment
Hide comment
@andrewdavidwong

andrewdavidwong Jul 8, 2017

Member

@marmarek, can you please enable this on the HCL subrepo?
Member

andrewdavidwong commented Jul 8, 2017

@marmarek, can you please enable this on the HCL subrepo?

@andrewdavidwong andrewdavidwong reopened this Jul 8, 2017

@andrewdavidwong andrewdavidwong assigned marmarek Jul 8, 2017

marmarek added a commit to QubesOS/qubes-hcl that referenced this issue Jul 11, 2017

@marmarek
travis: enable travis 
QubesOS/qubes-issues#2725
Verified
This commit was signed with a verified signature.
@marmarek marmarek Marek Marczykowski-Górecki
GPG key ID: F32894BE9684938A Learn about signing commits
ef7e23e
@marmarek

This comment has been minimized.

Show comment
Hide comment
@marmarek

marmarek Jul 11, 2017

Member

Done.
Member

marmarek commented Jul 11, 2017

Done.

@marmarek marmarek closed this Jul 11, 2017

@andrewdavidwong

This comment has been minimized.

Show comment
Hide comment
@andrewdavidwong

andrewdavidwong Jul 12, 2017

Member

Thanks, @marmarek!
Member

andrewdavidwong commented Jul 12, 2017

Thanks, @marmarek!

qubesos-bot-jpo pushed a commit to qubesos-bot-jpo/qubesos.github.io that referenced this issue Jul 22, 2017

@jpouellet
Differential spell checking 
This script does not attempt to find all misspellings, but rather show
when new words not previously known are introduced. This mean it's
useful in a continuous integration pipeline where we don't necessarily
want each contributor to have to be responsible for the bad spelling of
whoever came before them, but rather at least not make things worse.
The existing spelling can then be incrementally cleaned up.

This approach also means we don't need to maintain a local dictionary to
avoid false-positives, because existing "misspelled words" ("AppVM",
"marmarek", etc.) will be ignored.

In adition, effort is made to only check prose, not code. All the actual
parsing logic is handled by pandoc, and we just filter and transform a
JSON AST that it produces.

We also operate on the fully built HTML site rather than the source
markdown, meaning we see all spellings as they exist post-templating,
including all values from _data/*.yaml and such already filled in.

This was written primarily for use in a CI hook, but is of course also
useful locally. The default invocation checks the net difference between
origin/master and your local HEAD. You can also set SPELL_CHECK_CACHE to
a directory in which to keep results of previous runs, which speeds up
re-checking incremental changes.

Introduced dependencies are: hunspell (preferred) or aspell, and jq.

Fixes: QubesOS/qubes-issues#2725
Verified
This commit was signed with a verified signature.
@jpouellet jpouellet Jean-Philippe Ouellet
GPG key ID: E0C9723C50A46274 Learn about signing commits
cc32002

jpouellet added a commit to jpouellet/qubesos.github.io that referenced this issue Jul 22, 2017

@jpouellet
Differential spell checking 
This script does not attempt to find all misspellings, but rather show
when new words not previously known are introduced. This means it's
useful in a continuous integration pipeline where we don't necessarily
want each contributor to have to be responsible for the bad spelling of
whoever came before them, but rather at least not make things worse.
The existing spelling can then be incrementally cleaned up.

This approach also means we don't need to maintain a local dictionary to
avoid false-positives, because existing "misspelled words" ("AppVM",
"marmarek", etc.) will be ignored.

In addition, effort is made to only check prose, not code. All the
actual parsing logic is handled by pandoc, and we just filter and
transform a JSON AST that it produces.

We also operate on the fully built HTML site rather than the source
markdown, meaning we see all spellings as they exist post-templating,
including all values from _data/*.yaml and such already filled in.

This was written primarily for use in a CI hook, but is also useful
locally. It can be used in non-differential mode to show all existing
"misspellings" (with lots of false positives), or in differential mode
if you keep around an old copy of the built site. This only operates on
built copies of the site, so it is up to the caller to keep around an
old copy of the built site to compare against.

A previous version of this script which attempted to check out and build
the correct sites from git for comparison can be found here:
- QubesOS#110
It was determined that this was not the right way to approach the
problem.

Introduced dependencies are:
- hunspell (preferred) or aspell
- pandoc
- jq

Fixes: QubesOS/qubes-issues#2725
Verified
This commit was signed with a verified signature.
@jpouellet jpouellet Jean-Philippe Ouellet
GPG key ID: E0C9723C50A46274 Learn about signing commits
a67206d
@jpouellet

This comment has been minimized.

Show comment
Hide comment
@jpouellet

jpouellet Jul 22, 2017

Contributor

xref #2935 (qubesos.github.io: travis builds ~/old_site using merge head instead of base)
Contributor

jpouellet commented Jul 22, 2017

xref #2935 (qubesos.github.io: travis builds ~/old_site using merge head instead of base)

jpouellet added a commit to jpouellet/qubesos.github.io that referenced this issue Jul 24, 2017

@jpouellet
Differential spell checking 
This script does not attempt to find all misspellings, but rather show
when new words not previously known are introduced. This means it's
useful in a continuous integration pipeline where we don't necessarily
want each contributor to have to be responsible for the bad spelling of
whoever came before them, but rather at least not make things worse.
The existing spelling can then be incrementally cleaned up.

This approach also means we don't need to maintain a local dictionary to
avoid false-positives, because existing "misspelled words" ("AppVM",
"marmarek", etc.) will be ignored.

In addition, effort is made to only check prose, not code. All the
actual parsing logic is handled by pandoc, and we just filter and
transform a JSON AST that it produces.

We also operate on the fully built HTML site rather than the source
markdown, meaning we see all spellings as they exist post-templating,
including all values from _data/*.yaml and such already filled in.

This was written primarily for use in a CI hook, but is also useful
locally. It can be used in non-differential mode to show all existing
"misspellings" (with lots of false positives), or in differential mode
if you keep around an old copy of the built site. This only operates on
built copies of the site, so it is up to the caller to keep around an
old copy of the built site to compare against.

A previous version of this script which attempted to check out and build
the correct sites from git for comparison can be found here:
- QubesOS#110
It was determined that this was not the right way to approach the
problem.

Introduced dependencies are:
- hunspell (preferred) or aspell
- pandoc
- jq

Fixes: QubesOS/qubes-issues#2725
Verified
This commit was signed with a verified signature.
@jpouellet jpouellet Jean-Philippe Ouellet
GPG key ID: E0C9723C50A46274 Learn about signing commits
72b26cf

@jpouellet jpouellet referenced this issue in QubesOS/qubesos.github.io Jul 24, 2017

Merged

Spell checking redux #115

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment