Opened on 12/13/2016 at 04:19:58 AM

Closed on 09/26/2018 at 10:01:45 PM

#4729 closed defect (worksforme)

Packager Edge tests fail on Windows due to differing mime types

Reported by: oleksandr Assignee:
Priority: Unknown Milestone:
Module: Automation Keywords:
Cc: kvas, sebastian Blocked By:
Blocking: Platform: Edge
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Environment

Windows 10, Anniversary Update 14393.479

How to reproduce

  1. Run the tests: tox

Observed behaviour

The test_full_content_types_map() test fails since .xml is mapped to text/xml instead of application/xml and .json is mapped to application/javascript instead of application/json.

The same code works on Linux, so this is Windows specific.

Expected behaviour

The tests pass.

Attachments (0)

Change History (9)

comment:1 Changed on 03/02/2017 at 10:45:59 AM by kvas

After a brief search it seems that using application/javascript for JSON is not correct. Using text/xml for XML is fine, although it has slightly different meaning from (also allowed) application/xml (see RFC2376 section 3). I would say that we should work around the application/javascript issue by overriding the MIME type (the same way we did it for .otf in #4575) and we can leave XML as is and change the test or also override the MIME type.

I prefer adding two overrides and not changing the test so that our MIME type for XML would not depend on the platform on which the package was built. Ollie, what do you think?

comment:2 Changed on 03/02/2017 at 06:20:24 PM by oleksandr

Windows is a second class citizen all over our build tools. You can't really test sitescripts without heavily modifying the settings. I wonder if we should bother with this at all, seeing sitescripts aren't meant to be run on Windows?

comment:3 Changed on 03/02/2017 at 06:28:30 PM by kvas

Probably nobody but you would be building Edge extensions on Windows, so you tell me if this it's useful to fix this. It seems like not much work, but if you don't think it's useful enough, I'd be happy to close it as donotfix.

comment:4 follow-up: Changed on 03/02/2017 at 07:37:57 PM by oleksandr

I am fine using bash on Windows 10. So no need to fix for me. But I suppose we should mention somewhere in a README.md that this code is de-facto non-Windows compliant.

comment:5 in reply to: ↑ 4 Changed on 03/02/2017 at 07:46:35 PM by kvas

Replying to oleksandr:

I am fine using bash on Windows 10...

But wouldn't Python still behave the same way if you run it from Bash?

But I suppose we should mention somewhere in a README.md that this code is de-facto non-Windows compliant.

If it's just this one thing, I would rather fix it than explain in README.md why it doesn't work. It's almost easier ;)

But if there are other ways in which buildtools is not compatible with Windows, perhaps giving up would be easier. In any case, we'd need to wait for Sebastian to make this decision.

comment:6 follow-up: Changed on 03/02/2017 at 10:03:38 PM by oleksandr

Python behaves the same on Bash on Windows 10 as it does on Ubuntu. Since Bash on Windows 10 is essentially Ubuntu.

The problem on Windows-native is not just this one thing. For example tox.ini uses 'cp' command to copy files, which is not available on Windows. There's also some issue with pysed call after that, which I didn't really investigate. Then, the .sitescripts.example is relying on a mailer, which I guess can be replaced by XAMPP on Windows, but then it has to be in the README.md as well.

My point is just that if we are not testing on Windows we probably don't support it. I would be happy if we decided to support Windows, of course, but I see very little sense in that for the sitescripts module.

That being said, I would also be fine with adding two overrides to solve this particular issue too.

comment:7 in reply to: ↑ 6 Changed on 03/03/2017 at 02:26:51 PM by kvas

Replying to oleksandr:

Python behaves the same on Bash on Windows 10 as it does on Ubuntu. Since Bash on Windows 10 is essentially Ubuntu.

Oh, I didn't know about that. Pretty cool!

Anyway, since just fixing this one thing still doesn't make buildtools Windows-compatible and since you don't really need it to be, I would say I prefer updating the README. Let's wait for Sebastian to be back and see what he thinks.

comment:8 Changed on 03/13/2017 at 01:08:13 PM by sebastian

Generally, it should be as simple and easy as possible to get started working on Adblock Plus, also keeping external contributors in mind. Also considering that we now support Microsoft Edge which you would naturally develop for on Windows. While on current Windows 10, you can easily get the Ubuntu/Linux userland, it is still some additional steps, that we might want to avoid, for buildtools at least. However, some comments above seem to refer to sitescripts which is a different story.

comment:9 Changed on 09/26/2018 at 10:01:45 PM by sebastian

  • Resolution set to worksforme
  • Status changed from new to closed

This is no longer relevant since we switched to manifoldjs.

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.