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

Fix tests and improve CI #590

Merged
merged 12 commits into from Dec 17, 2018
Merged

Fix tests and improve CI #590

merged 12 commits into from Dec 17, 2018

Conversation

fxha
Copy link
Contributor

@fxha fxha commented Dec 3, 2018

Q A
Bug fix? yes
New feature? yes
Fixed tickets #288
License MIT

Description

  • Update unit and e2e test packages
  • Fix tests
  • Enable virtual screen (xvfb) on Linux
  • Fix mocha timeout when using a non-interactive terminal
  • Custom TOC parser because of a markdown-toc issue
    • Create markdown TOC while parsing the document
    • Slugify headings
  • Add a full-featured markdown template to test against

resolves #288

@fxha fxha force-pushed the tests branch 2 times, most recently from 4c8cd98 to 4bcce27 Compare December 3, 2018 14:41
@fxha
Copy link
Contributor Author

fxha commented Dec 10, 2018

@Jocs Can you please take a look at this error message. I got the same error with node v8.10 in the docker container but after using v8.14 (LTS) the error was gone. OK, it's an issue with a dependency because I can reproduce the error with yarn but not with npm and package-lock.json. However, it's a problem. There is also a mocha e2e test timeout when using a non-interactive terminal which seems to be a problem with starting the browser.

[...]
  Launch
    1) "before each" hook: beforeEach for "shows the proper application title"


  0 passing (30s)
  1 failing

  1) Launch
       "before each" hook: beforeEach for "shows the proper application title":
     Error: Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/dev/marktext/test/e2e/index.js)

If someone need a dockerfile:

FROM ubuntu:18.04

ENV DEBIAN_FRONTEND noninteractive
ENV DEBCONF_NONINTERACTIVE_SEEN true
ENV TZ UTC

RUN apt-get update -y && apt-get install -y -q apt-transport-https apt-utils software-properties-common ca-certificates curl wget

RUN curl -sL https://deb.nodesource.com/setup_8.x | bash -
RUN apt-get update -y && apt-get install -y -q nodejs

# RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add -
# RUN apt-add-repository "deb https://dl.yarnpkg.com/debian/ stable main"
# RUN apt-get update -y && apt-get install -y -q yarn

RUN apt-get update -y
RUN apt-get install --no-install-recommends -y -q \
  git make clang \
  libgconf2-dev libnss3 libasound2 libxtst-dev libxss1 libnotify4 libxtst6 xdg-utils libglib2.0-bin gvfs-bin \
  libgnome-keyring-dev icnsutils graphicsmagick xz-utils xvfb libx11-dev libxkbfile-dev \
  curl firefox

ENV CC clang
ENV CXX clang++
ENV npm_config_clang 1
# ENV PATH $PATH:/opt/yarn/bin

RUN $CC --version
RUN $CXX --version
RUN node --version
# RUN yarn --version
RUN npm --version

ENV MARKTEXT_IS_OFFICIAL_RELEASE 1
ENV MARKTEXT_EXIT_ON_ERROR 1

# TODO: clone git repo
ADD marktext /home/dev/marktext
WORKDIR /home/dev/marktext

RUN npm install

#RUN npm run lint

ENV DISPLAY :99.0
RUN xvfb-run --server-args="-screen 0 1024x768x24" npm run test

@fxha fxha added 🛑 blocked This issue or PR is blocked by other issue and removed 🛑 blocked This issue or PR is blocked by other issue labels Dec 11, 2018
@fxha
Copy link
Contributor Author

fxha commented Dec 13, 2018

Very strange, markdown-toc, which was a short-term solution, was the reason why the tests don't pass on Travis CI but on AppVeyor and Desktop Linux. I removed it and will replace it with a custom muya implementation.

@Jocs BTW we have different application title on macOS Mark Text and Linux/Window the document name.

@fxha fxha requested a review from Jocs December 15, 2018 18:10
@fxha fxha changed the title [WIP] Fix tests and improve CI Fix tests and improve CI Dec 15, 2018
const blocks = ctx.contentState.getBlocks()
const exportedMarkdown = new ExportMarkdown(blocks).generate()

// FIXME: We always need to add a new line at the end of the document. Add a option to disable the new line.
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean we need add an option to disable add new line when generating the markdown text? or we don't need to add new line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a cool feature to insert a new line at the end of a document but it should exists an option to disable it. I had troubles with pure strings like const s = '# heading' initially because I had to insert a new line to each test.

@Jocs
Copy link
Member

Jocs commented Dec 16, 2018

Add a full-featured markdown template to test against

Great work!

Enable virtual screen (xvfb) on Linux

What's virtual screen?

@Jocs
Copy link
Member

Jocs commented Dec 16, 2018

I removed it and will replace it with a custom muya implementation.

Nice work!

@fxha
Copy link
Contributor Author

fxha commented Dec 16, 2018

What's virtual screen?

Xvfb or X virtual framebuffer is a display server implementing the X11 display server protocol. In contrast to other display servers, Xvfb performs all graphical operations in virtual memory without showing any screen output. [...] This virtual server does not require the computer it is running on to have any kind of graphics adapter, a screen or any input device. Only a network layer is necessary.

Basically it allows us the run GUI applications on a machine with no display and no physical input devices. macOS and Windows support these "virtual screens" out of the box.


@Jocs During the muya rewrite you should create a TOC list during parsing the document. My solution is not implemented well because we parse the document twice when the TOC sidebar is opened - same as the previous mardown-toc implementation.

@Jocs
Copy link
Member

Jocs commented Dec 17, 2018

During the muya rewrite you should create a TOC list during parsing the document. My solution is not implemented well because we parse the document twice when the TOC sidebar is opened - same as the previous mardown-toc implementation.

OK, I'll take it into consideration.

@Jocs Jocs merged commit 2ce0582 into marktext:master Dec 17, 2018
@fxha fxha deleted the tests branch December 17, 2018 10:14
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.

None yet

2 participants