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

Visual testing on Asciidoctor User Manual #30

Closed
wants to merge 1 commit into from

Conversation

ggrossetie
Copy link
Member

@ggrossetie ggrossetie commented Apr 4, 2016

Install and testing procedure:

 $ npm install
 $ casperjs test spec/testsuite.js

Generate reference screenshots:

 $ rm -f spec/screenshots/*
 $ asciidoctor spec/fixtures/docs/user-manual-intro.adoc

Generate user manual with the latest stylesheet:

 $ asciidoctor -a stylesheet=../../../asciidoctor.min.css spec/fixtures/docs/user-manual-intro.adoc

NOTE: Do not forget to unable Google Fonts (ie. uncomment the @import)

Run test again:

 $ casperjs test spec/testsuite.js

Currently PhantomCSS hangs if I run the visual testing against the complete user-manual.html (ie. with the content div). Maybe PhantomCSS doesn't like id duplications (see asciidoctor/asciidoctor.org#565) or maybe the document is too big to be analyzed within reasonable time.

TODO

  • Add install and testing procedure to the README
  • Update user-manual.html with the latest stylesheet
  • Fix footer date to avoid regression

@ggrossetie
Copy link
Member Author

@mojavelinux In the intro section of the User Manual, the only visual "regression" are:

  • Padding on admonition block is slightly different (space between the icon and text)
  • Responsive breakpoints are not exactly the same (with a 1024px*768px viewport)

Differences in pink:
user manual content_2 fail

I changed viewport to 1920px*1080px

@ggrossetie
Copy link
Member Author

@mojavelinux Could you please enable Travis build ? I want to know if the build is working on Travis ☺️

@mojavelinux
Copy link
Member

Yep! I can take care of that.

@mojavelinux
Copy link
Member

Enabled. I think you need to force push to trigger the build.

@mojavelinux
Copy link
Member

This will be very important for me to get back to the work to upgrade us to Foundation 5/6. That requires changing from ems to rems in a lot of places and that introduces to room for a lot of subtle, hard to detect changes.

@ggrossetie
Copy link
Member Author

ggrossetie commented Sep 9, 2017

cssshrink is incompatible with the latest version of color:

stoyan/cssshrink#33
stoyan/csscolormin#6

EDIT: Another way to fix this issue is to find out which of the color* librairies is compatible with cssshrink. I tried but I think it's better to upgrade 😉

@mojavelinux
Copy link
Member

I'd rather lock the version. I really don't want to spend a lot of time on upgrading tools since we might change the way we build the stylesheet anyway. That's where the visual testing comes in.

@ggrossetie
Copy link
Member Author

I'd rather lock the version. I really don't want to spend a lot of time on upgrading tools since we might change the way we build the stylesheet anyway. That's where the visual testing comes in.

@mojavelinux That was my initial plan but I couldn't figure out what version of color and color-string is compatible with csscolormin. I will try again to find a compatible version. If cssshrink is working on your machine, could you please give me the output of npm list--depth=0 ?

I think we should use yarn "lockfile"... so that, even if an author of a Node module does not use specific version we should have a reliable build ! 🚀

@mojavelinux
Copy link
Member

Here's the output you requested:

└─┬ [email protected]
  ├─┬ [email protected]
  │ ├─┬ [email protected]
  │ │ ├── [email protected]
  │ │ ├── [email protected]
  │ │ └── [email protected]
  │ └─┬ [email protected]
  │   └── [email protected]
  ├─┬ [email protected]
  │ └── [email protected]
  └── [email protected]

I think we should use yarn "lockfile".

100%

@ggrossetie ggrossetie force-pushed the phantom-css branch 3 times, most recently from 60ee864 to 1ac33d3 Compare September 17, 2017 16:49
@ggrossetie
Copy link
Member Author

@mojavelinux Thanks, the build is now running fine, but the screenshots are slightly different.
I think we need to upload the generated screenshots somewhere... maybe we could use GitHub pages ?

@mojavelinux
Copy link
Member

We're going to use backstopjs to regression test the default stylesheet instead. See asciidoctor/asciidoctor#4070

@mojavelinux mojavelinux closed this Aug 7, 2021
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