Skip to content
This repository was archived by the owner on Sep 10, 2025. It is now read-only.

Conversation

@Nayef211
Copy link
Contributor

@Nayef211 Nayef211 commented Aug 31, 2022

Description

  • Introduces test/torchtext_unittest directory and moves all the test scripts and assets there (this is in line with how torchaudio organizes it's unit tests)
  • changes python setup.py develop to python setup.py install in CI job
  • run test with (cd test && pytest torchtext_unittest) so that repository root is not in Python module search path.
  • Change relative import (from ..common import ...) in test module to absolute import (from torchtext_unittest.common import ...)
  • Creates a workflow to run integration tests seperately from unittests

Additional context

Followups

Closes #1875

@Nayef211 Nayef211 changed the title Create new torchtext_unittest folder Fix test execution Sep 1, 2022
@Nayef211 Nayef211 changed the title Fix test execution Fix test execution in torchtext Sep 1, 2022
@Nayef211 Nayef211 requested a review from mthrok September 9, 2022 19:49
@Nayef211 Nayef211 marked this pull request as ready for review September 9, 2022 19:49
@Nayef211 Nayef211 requested a review from joecummings September 9, 2022 19:49
@joecummings
Copy link
Member

To confirm: Failures related to test_vocab_from_raw_text_file will be resolved in a followup PR or in this PR?

@Nayef211
Copy link
Contributor Author

Nayef211 commented Sep 12, 2022

To confirm: Failures related to test_vocab_from_raw_text_file will be resolved in a followup PR or in this PR?

The plan is to investigate and resolve them in a followup PR. There aren't any significant changes in this PR that should cause that test to fail.

Copy link
Member

@joecummings joecummings left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

Overall it looks good.

@mthrok
Copy link
Contributor

mthrok commented Sep 12, 2022

To confirm: Failures related to test_vocab_from_raw_text_file will be resolved in a followup PR or in this PR?

The plan is to investigate and resolve them in a followup PR. There aren't any significant changes in this PR that should cause that test to fail.

Was it failing before?

@Nayef211
Copy link
Contributor Author

To confirm: Failures related to test_vocab_from_raw_text_file will be resolved in a followup PR or in this PR?

The plan is to investigate and resolve them in a followup PR. There aren't any significant changes in this PR that should cause that test to fail.

Was it failing before?

Yes it was failing sporadically on certain python versions on Linux machines. Disabled them for the time being in #1901

@Nayef211
Copy link
Contributor Author

Nayef211 commented Sep 15, 2022

Deleted the file

  • Potential follow-up: keeping asset dir and make integration test not access internals of torchtext_unittest would make it cleaner.

Added it as a followup item in PR description

@Nayef211 Nayef211 merged commit 67d2692 into pytorch:main Sep 16, 2022
@Nayef211 Nayef211 deleted the test/create_new_unittest_folder branch September 16, 2022 00:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error running unit tests when building with setup.py install

4 participants