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

Switch to upstream html5lib-tests #2287

Merged
merged 1 commit into from
Jul 9, 2021
Merged

Switch to upstream html5lib-tests #2287

merged 1 commit into from
Jul 9, 2021

Conversation

stevecheckoway
Copy link
Contributor

My PR to fix the errors
html5lib/html5lib-tests#136 was merged. We can
now track the upstream repository.

Closes #2282

What problem is this PR intended to solve?

The html5 tests had been tracking my fork of html5lib-tests but with html5lib/html5lib-tests#136 merged, we can track upstream. #2282

Have you included adequate test coverage?

There is no change in test coverage.

Does this change affect the behavior of either the C or the Java implementations?

This is only relevant to the C implementation, but it affects the behavior of neither.

@stevecheckoway
Copy link
Contributor Author

stevecheckoway commented Jul 9, 2021

I'm not completely sure I correctly updated the submodule. Hopefully the CI run will make it clear which commit from which repo the submodule was checked out.

And it looks good. ``` Fetching submodules /usr/bin/git submodule sync /usr/bin/git -c protocol.version=2 submodule update --init --force --depth=1 Submodule 'test/html5lib-tests' (https://github.com/html5lib/html5lib-tests.git) registered for path 'test/html5lib-tests' Cloning into '/home/runner/work/nokogiri/nokogiri/test/html5lib-tests'... From https://github.com/html5lib/html5lib-tests * branch 535e74b4759d94fdc4038d2da9d6b70da6287614 -> FETCH_HEAD Submodule path 'test/html5lib-tests': checked out '535e74b4759d94fdc4038d2da9d6b70da6287614' ```

No, that was the wrong commit. This time it looks fine.

My PR to fix the errors
html5lib/html5lib-tests#136 was merged. We can
now track the upstream repository.

Closes #2282
@flavorjones
Copy link
Member

@stevecheckoway Would it be useful to have a job in the "upstream" CI pipeline to run tests against the latest head of html5lib/html5lib-tests?

That pipeline runs a few times a week on a timer to give us some advanced notice if upstream changes are causing test failures.

If that seems useful, I'm happy to add the job to this PR or create a separate one, whichever you'd prefer.

@flavorjones
Copy link
Member

Ah, I'm going to merge this and we can chat about an upstream job in a new PR.

@flavorjones flavorjones merged commit ab6c3e4 into sparklemotion:main Jul 9, 2021
@stevecheckoway stevecheckoway deleted the switch-tests branch July 9, 2021 16:15
@stevecheckoway
Copy link
Contributor Author

Such an upstream job sounds like a fantastic idea!

@flavorjones
Copy link
Member

See #2288 for upstream PR.

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.

Update html5lib-tests test to official repo
2 participants