Appveyor: Problem with dependencies #1425

Techievena · 2017-02-10T02:36:08Z

XMLBear is skipped on Appveyor Windows builds
due to dependency issues. Fixes all dependency
issues.

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

I read the commit guidelines and I've followed
them.
I ran coala over my code locally. (All commits have to pass
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use cobot mark wip <URL> to get it out
of the review queue.

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

AsnelChristian · 2017-02-10T15:44:04Z

Your commit message isn't clear, would you improve on that ?

AsnelChristian · 2017-02-10T15:46:15Z

i am not sure why, your tests failed both on CI and travis ....

AsnelChristian · 2017-02-10T15:50:18Z

oooh, it seems XMLBearTest are still being skipped on appveyor

Techievena · 2017-02-10T18:43:19Z

@AsnelChristian Referred this website https://atom.io/packages/linter-xmllint and @jayvdb asked to install packages using nuget. Did I miss anything?

adtac · 2017-02-15T11:34:47Z

.ci/appveyor.yml

@@ -56,6 +56,15 @@ install:
  - "npm config set loglevel warn"
  - "npm install"

+  - curl -fsSL ftp://ftp.zlatkovic.com/libxml/iconv-1.9.2.win32.zip -o iconv.zip


Does Windows even have curl and 7z installed by default?

@adtac It is installing the packages correctly, curl and 7z must be installed by default I guess

https://ci.appveyor.com/project/coala/coala-bears/build/1.0.5260/job/34vesuyausnmwate#L654

Appveyor has lots of non-standard Windows software installed, especially msys & cygwin. See https://www.appveyor.com/docs/installed-software/

Though it is probably quicker to use wget "ftp://ftp.zlatkovic.com/libxml/*.win32.zip" to batch download the files, and 7z may also have a batch unzip mode.

~~Please don't user curl. It caused problems in another place: coala/coala#3962. Use the builtin DownloadFile~~

Disregard above comment^.

jayvdb · 2017-02-15T23:28:20Z

Circle has been fixed with #1434 ; please rebase.

jayvdb

Why is this "review pending"?
On both 32bit and 64bit Appveyor builds, all xml tests are skipped.

jayvdb · 2017-02-16T23:32:39Z

.ci/appveyor.yml

@@ -29,7 +29,7 @@ install:
  # Prepend newly installed Python to the PATH of this build (this cannot be
  # done from inside the powershell script as it would require to restart
  # the parent CMD process).
-  - "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%"
+  - "SET PATH=C:\\tools\\xmllint\\bin;%PYTHON%;%PYTHON%\\Scripts;%PATH%"


This addition should be on the next line.
The reason is that this line is identical to https://github.com/ogrisel/python-appveyor-demo/blob/f54ec35/appveyor.yml#L103

jayvdb · 2017-02-16T23:47:18Z

.ci/appveyor.yml

@@ -56,6 +56,15 @@ install:
  - "npm config set loglevel warn"
  - "npm install"

+  - curl -fsSL ftp://ftp.zlatkovic.com/libxml/iconv-1.9.2.win32.zip -o iconv.zip


Though it is probably quicker to use wget "ftp://ftp.zlatkovic.com/libxml/*.win32.zip" to batch download the files, and 7z may also have a batch unzip mode.

Techievena · 2017-02-17T04:56:38Z

@jayvdb I added the same dependencies for xmllint as mentioned in https://atom.io/packages/linter-xmllint . I really can't figure out why the added dependencies are not sufficient. I'd do the fixes you suggested but I don't think it'll be of any need because you said earlier to install the dependencies via NuGet. I did in this manner just to ensure that the dependencies I am using are correct for xmllint. But the tests are still getting skipped. Don't know what is missing here? 😟

Techievena · 2017-02-20T16:29:50Z

@jayvdb wget is not available in Appveyor. We have to download it. Should I go for it?

gitmate-bot · 2017-04-19T07:48:13Z

tests/xml2/XMLBearTest.py


 schema_file = os.path.join(os.path.dirname(__file__),
                           'test_files',
-                           'note.xsd')
+                           'note.xsd').replace('\\','\\\\')


E231 missing whitespace after ',''

PycodestyleBear (E231), severity NORMAL, section autopep8.

gitmate-bot · 2017-04-19T07:48:13Z

tests/xml2/XMLBearTest.py

@@ -38,11 +38,11 @@ def load_testdata(filename):

 dtd_file = os.path.join(os.path.dirname(__file__),
                        'test_files',
-                        'note.dtd')
+                        'note.dtd').replace('\\','\\\\')


E231 missing whitespace after ',''

PycodestyleBear (E231), severity NORMAL, section autopep8.

gitmate-bot · 2017-04-19T07:48:14Z

tests/xml2/XMLBearTest.py


 schema_file = os.path.join(os.path.dirname(__file__),
                           'test_files',
-                           'note.xsd')
+                           'note.xsd').replace('\\','\\\\')


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/tests/xml2/XMLBearTest.py +++ b/tests/xml2/XMLBearTest.py @@ -42,7 +42,7 @@ schema_file = os.path.join(os.path.dirname(__file__), 'test_files', - 'note.xsd').replace('\\','\\\\') + 'note.xsd').replace('\\', '\\\\') valid_xml_path = load_testdata('note.xml') valid_xml_url = load_testdata('concept-valid.xml')

gitmate-bot · 2017-04-19T07:48:15Z

tests/xml2/XMLBearTest.py

@@ -38,11 +38,11 @@ def load_testdata(filename):

 dtd_file = os.path.join(os.path.dirname(__file__),
                        'test_files',
-                        'note.dtd')
+                        'note.dtd').replace('\\','\\\\')


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/tests/xml2/XMLBearTest.py +++ b/tests/xml2/XMLBearTest.py @@ -38,7 +38,7 @@ dtd_file = os.path.join(os.path.dirname(__file__), 'test_files', - 'note.dtd').replace('\\','\\\\') + 'note.dtd').replace('\\', '\\\\') schema_file = os.path.join(os.path.dirname(__file__), 'test_files',

gitmate-bot · 2017-04-26T17:07:21Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

XMLBear is skipped on Appveyor Windows builds due to dependency issues. Fixes all dependency issues. Closes coala#1354

jayvdb · 2017-05-04T09:19:36Z

Please remove the escape, so that the problem can be seen in the build logs.

You can add a comment to describe your workaround, which is quite a neat trick I must say.
But somehow, we need to find the real cause, and plug it, otherwise we do not support Windows properly.

The call chain (ignoring the test framework) is

XMLBear.create_arguments defines arguments xml_schema as Setting.path, and xml_dtd as path_or_url (and that calls Settings.path).
Settings.__path__ calls str(self), and boom ... unescaping occurs.

I believe coala/coala#4171 will fix the problem, but I am way out of my depth there, so I have asked @adtac to review.

There is also a chance that somewhere in the test framework, the Setting is being created incorrectly.

jayvdb · 2017-05-05T01:44:22Z

Another update at coala/coala#4171 (comment)

gitmate-bot · 2017-05-12T17:03:55Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-19T15:44:02Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-27T01:30:49Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-03T00:10:21Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-10T09:57:11Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-17T08:36:58Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-24T07:17:25Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-01T05:57:15Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-08T04:37:04Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-15T03:17:23Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-22T01:57:38Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot added process/pending review size/S labels Feb 10, 2017

AsnelChristian added process/wip and removed process/pending review labels Feb 10, 2017

Techievena force-pushed the appveyor branch from 5e2c1f9 to 6dcf141 Compare February 11, 2017 04:48

gitmate-bot added process/pending review size/XS and removed process/wip size/S labels Feb 11, 2017

Techievena force-pushed the appveyor branch 2 times, most recently from a810dab to 465613f Compare February 11, 2017 05:17

adtac reviewed Feb 15, 2017

View reviewed changes

Techievena force-pushed the appveyor branch from 465613f to 463f1d9 Compare February 16, 2017 08:08

jayvdb requested changes Feb 16, 2017

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Feb 16, 2017

Techievena force-pushed the appveyor branch from 463f1d9 to cd6470d Compare February 20, 2017 15:54

gitmate-bot added process/pending review and removed process/wip labels Feb 20, 2017

Techievena force-pushed the appveyor branch 2 times, most recently from fe46e4f to cea5cbf Compare February 20, 2017 16:20

gitmate-bot added size/S and removed size/XS labels Feb 20, 2017

gitmate-bot added size/S and removed size/XS labels Apr 19, 2017

Techievena force-pushed the appveyor branch 2 times, most recently from 5e13e51 to 99e908a Compare April 19, 2017 07:47

gitmate-bot reviewed Apr 19, 2017

View reviewed changes

Techievena force-pushed the appveyor branch from 99e908a to 337b4e6 Compare April 19, 2017 07:55

AppveyorCI: Prevent skip of tests of XMLBear

7b35d49

XMLBear is skipped on Appveyor Windows builds due to dependency issues. Fixes all dependency issues. Closes coala#1354

Techievena force-pushed the appveyor branch from 337b4e6 to 7b35d49 Compare May 2, 2017 07:37

jayvdb mentioned this pull request May 4, 2017

Setting.__path__: Use Setting.value instead of str(Setting) coala/coala#4171

Closed

jayvdb added process/wip and removed process/pending review labels May 4, 2017

jayvdb mentioned this pull request May 9, 2017

Setting.__path__ allows use with strings coala/coala#4201

Open

gitmate-bot added the status/STALE label Aug 31, 2017

Appveyor: Problem with dependencies #1425

Are you sure you want to change the base?

Appveyor: Problem with dependencies #1425

Conversation

Techievena commented Feb 10, 2017

Checklist

AsnelChristian commented Feb 10, 2017

AsnelChristian commented Feb 10, 2017

AsnelChristian commented Feb 10, 2017

Techievena commented Feb 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mixih Mar 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb commented Feb 15, 2017

jayvdb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Techievena commented Feb 17, 2017 • edited Loading

Techievena commented Feb 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitmate-bot commented Apr 26, 2017

jayvdb commented May 4, 2017

jayvdb commented May 5, 2017

gitmate-bot commented May 12, 2017

gitmate-bot commented May 19, 2017

gitmate-bot commented May 27, 2017

gitmate-bot commented Jun 3, 2017

gitmate-bot commented Jun 10, 2017

gitmate-bot commented Jun 17, 2017

gitmate-bot commented Jun 24, 2017

gitmate-bot commented Jul 1, 2017

gitmate-bot commented Jul 8, 2017

gitmate-bot commented Jul 15, 2017

gitmate-bot commented Jul 22, 2017

Techievena commented Feb 10, 2017 •

edited

Loading

Mixih Mar 21, 2017 •

edited

Loading

Techievena commented Feb 17, 2017 •

edited

Loading