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

Folder structure redone for tests #1064

Merged
merged 4 commits into from
Aug 23, 2023
Merged

Folder structure redone for tests #1064

merged 4 commits into from
Aug 23, 2023

Conversation

Rishabhg71
Copy link
Collaborator

closes #1062

@Rishabhg71
Copy link
Collaborator Author

Rishabhg71 commented Aug 22, 2023

I have left out tests/unit as it is. Do you think I should do any refactoring over there I mean there is a single and single file with the test. If you think it should be done I could come up with some structure as well
@Jaifroid

@Jaifroid
Copy link
Member

Happy for you to propose something, or just structure the e2e tests, as you wish!

@Rishabhg71
Copy link
Collaborator Author

Rishabhg71 commented Aug 23, 2023

@Jaifroid I think its good enough to merge now then

Copy link
Member

@Jaifroid Jaifroid left a comment

Choose a reason for hiding this comment

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

It looks good, and much more organized, thank you! There's one small change (see comment).

tests/foo-zstd.zim Show resolved Hide resolved
@Jaifroid
Copy link
Member

Many thanks for this PR. Squashing/merging now.

@Jaifroid Jaifroid merged commit 632f019 into kiwix:main Aug 23, 2023
6 checks passed
@Jaifroid
Copy link
Member

@RG7279805 Unfortunately, it looks like this PR broke the BrowserStack implementation, and of course that didn't get tested in your testing of it, because BrowserStack tests can't run with PRs from forked repositories. I'm re-running the tests, but we might have to revert. I'm also getting failures on my internationalization branch that appear to be caused by the PR.

@Jaifroid
Copy link
Member

Hmm, definitely failing, check out:

https://github.com/kiwix/kiwix-js/actions/runs/5951851073/job/16149678973 and https://github.com/kiwix/kiwix-js/actions/runs/5953037189/job/16146376792?pr=1061.

I tried to fix the paths for the legacy edge runner on my branch, but it still fails.

Jaifroid added a commit that referenced this pull request Aug 23, 2023
@Jaifroid
Copy link
Member

So, I've reverted for now, but your PR is saved at this commit:

632f019

It probably needs some adjustment, but I can't seem to work out what at the moment, and you will have a clearer idea of what changed.

@Rishabhg71
Copy link
Collaborator Author

I think then I should create a PR fixing #1062 , #1063 and also making sure everything runs smoothly

@Jaifroid
Copy link
Member

That would be great. Please don't worry about this small setback -- you couldn't have known because the tests weren't run and you don't have access to the BrowserStack setup yet. As soon as I've merged the internationalization branch, I'll turn to working out best way to give you access. Perhaps you could send me a slack message with the best email address for me to invite you to be a member of the team able to view test videos etc. on BrowserStack.

@Rishabhg71
Copy link
Collaborator Author

I have sent you a message on Slack until then I will wait

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.

Folder structure for test and proper file segregation
2 participants