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

skip building test harness in docker image #551

Merged
merged 6 commits into from
Jun 30, 2023

Conversation

paulfitz
Copy link
Member

@paulfitz paulfitz commented Jun 29, 2023

A small test harness bundle was recently added that is breaking the docker image build. It could be added to the docker image, but that would introduce a bunch of extraneous test file dependencies. So this tweaks the build to simply skip the test bundle if its primary source file is not found.

Also added some other test fixes along the way:

  • make a custom widget test more reliable
  • update a localization test now that pt exists
  • store more log info in artifact on error

@paulfitz
Copy link
Member Author

The relevant docker build actions are here: https://github.com/gristlabs/grist-core/actions/workflows/docker_latest.yml

A small test harness bundle was recently added that is breaking
the docker image build. It could be added to the docker image,
but that would introduce a bunch of extraneous test file
dependencies. So this tweaks the build to simply skip the test
bundle if its primary source file is not found.
@paulfitz paulfitz force-pushed the paulfitz/omit-test-harness-from-docker-build branch from ce096f5 to 3f959e3 Compare June 30, 2023 06:13
@paulfitz
Copy link
Member Author

paulfitz commented Jun 30, 2023

Example of artifact on a failing run https://github.com/paulfitz/grist-core/actions/runs/5419990087 - could do with a better way to cross reference worker process number with mocha-webdriver output, and I'm not sure the mocha-webdriver textual output is complete, but that'll be for another PR.

document.getElementById('configure').innerHTML = 'called';
};
}
function setup() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why that helped? It doesn't seem to change anything (and we have custom widgets that follow the previous pattern).

Copy link
Member Author

Choose a reason for hiding this comment

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

This way, the setup is called only when the page has loaded and the elements it refers to exist. Previously, there was a race - if callbacks happened too early, elements like configure could be accessed before they existed.

@paulfitz paulfitz merged commit 70935a4 into main Jun 30, 2023
@paulfitz paulfitz deleted the paulfitz/omit-test-harness-from-docker-build branch June 30, 2023 09:50
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.

2 participants