search

LeaVerou / css3test

How does your browser score for its CSS3 support?
http://css3test.com
MIT License
214 stars 83 forks source link

Split all tests into separate files #227

Closed SebastianZ closed 2 years ago

SebastianZ commented 2 years ago

While splitting them they were also converted into ES modules.

This also came with some consequences. The Array prototype extensions times() and and() got removed and the tests that used them now include the complete arrays previously produced by those methods. Furthermore, the ele() function got replaced by Bliss' $() function.

I probably could have moved them into some kind of helper module, though I believe those functions aren't necessary.

I only turned the tests into modules in this change, as I focused on #219. Though other parts of the code should probably also be converted into modules (as part of #216).

This change will cause conflicts with the other PRs, so depending on which ones are merged first, some rebasing needs to happen.

This closes #219.

Sebastian

LeaVerou commented 2 years ago

Beautiful.

Zefling commented 2 years ago

It's a shame that with this mode, it's no longer stand alone (file://) and that we need a server to test.

SebastianZ commented 2 years ago

It's a shame that with this mode, it's no longer stand alone (file://) and that we need a server to test.

Right, though requiring a server shouldn't be a blocker for this. If you don't have or want to run a full-fledged server, there are also various ways to start a server from within your IDE. E.g. in VSCode there's a plugin called Live Server, which allows you to start a server for the currently open folder with a single click.

@Zefling Is that your only concern or do you have other feedback?

Sebastian

Zefling commented 2 years ago

It's just an observation. Maybe write a note in the readme.

No problem other than that.

LeaVerou commented 2 years ago

Importing the same module twice does not fire a separate HTTP request. Also, isn't the whole point of HTTP/2 that multiple HTTP requests are fine? 😄

Side note: do we want to use spaces for indentation and CRLF line endings in this project now?

Oof, let's not please. I must have missed that when reviewing the PR.

SebastianZ commented 2 years ago

I understand re-exporting all the specs from one place (tests.js) is convenient for legibility but i think this is far from being ideal from a performance point of view, because this means these 105 (!) imports are actually discovered by the browser after two consecutive subrequests: csstest.js, and then tests.js.

The easier way i see to improve that is to import the specs directly from csstest.js.

tests.js is just one additional request, so it doesn't make much of a change to remove it. I could remove it, though I kept it for readablility and to keep the Specs structure unchanged.

Or we could preload some (all?) the files with <link rel="modulepreload">, but the support for it is not great currently.

I agree we should add <link rel="modulepreload">s. In browsers that don't support it it's just ignored, so there's no harm in adding it. I'd add one for all tests except the CSS 2 ones because they are excluded by default.

Side note: do we want to use spaces for indentation and CRLF line endings in this project now? 😅 I don't really care, but i would just prefer consistency across the project.

That's a mistake. My IDE wasn't set up correctly. I'll change the indentations back.

Sebastian

SebastianZ commented 2 years ago

Or we could preload some (all?) the files with <link rel="modulepreload">, but the support for it is not great currently.

I agree we should add <link rel="modulepreload">s. In browsers that don't support it it's just ignored, so there's no harm in adding it. I'd add one for all tests except the CSS 2 ones because they are excluded by default.

I've tested that now in Chrome 97 and Firefox 96 and didn't see any performance gain. Therefore I do not add them. @PolariTOON If you think we can somehow get a performance improvement by preloading resources, I suggest you open a separate issue and we'll discuss it there.

Side note: do we want to use spaces for indentation and CRLF line endings in this project now? 😅 I don't really care, but i would just prefer consistency across the project.

That's a mistake. My IDE wasn't set up correctly. I'll change the indentations back.

The indentation is fixed now.

Sebastian

PolariTOON commented 2 years ago

Also, I forgot to tell it above, but filter.js and files under tests/ still use Windows line endings.

SebastianZ commented 2 years ago

Also, I forgot to tell it above, but filter.js and files under tests/ still use Windows line endings.

Thank you for the hint! Line breaks should be Linux line endings all over the place now.

Sebastian

SebastianZ commented 2 years ago

The total number of features tested appears to have decreased, because the following specs are not exported by tests.js:

[
  "Box Sizing Level 3",
  "Box Sizing Level 4",
  "Custom Properties for Cascading Variables Level 1",
  "Scoping Level 1",
  "Scroll Anchoring Level 1",
  "Scroll Snap Level 1",
  "Scrollbars Level 1",
  "Shadow Parts",
  "Shapes Level 1",
  "Shapes Level 2",
  "Will Change Level 1",
  "Writing Modes Level 3",
  "Writing Modes Level 4"
]

Total fail by me. 😞 Thank you so much for checking! I've included them now.

Sebastian